-
Notifications
You must be signed in to change notification settings - Fork 15k
[clang][CodeGen][MSVC] Match how MSVC returns vector types from member functions #157365
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
8f42b86
fd8dd19
5970180
017bfea
c0f4b22
c6b0cd3
71cd7fd
2964b82
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1168,15 +1168,23 @@ static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty, | |
| } | ||
|
|
||
| bool MicrosoftCXXABI::classifyReturnType(CGFunctionInfo &FI) const { | ||
| const CXXRecordDecl *RD = FI.getReturnType()->getAsCXXRecordDecl(); | ||
| if (!RD) | ||
| return false; | ||
|
|
||
| bool isTrivialForABI = RD->canPassInRegisters() && | ||
| isTrivialForMSVC(RD, FI.getReturnType(), CGM); | ||
|
|
||
| // MSVC always returns structs indirectly from C++ instance methods. | ||
| bool isIndirectReturn = !isTrivialForABI || FI.isInstanceMethod(); | ||
| bool isIndirectReturn = false; | ||
| if (const CXXRecordDecl *RD = FI.getReturnType()->getAsCXXRecordDecl()) { | ||
| bool isTrivialForABI = RD->canPassInRegisters() && | ||
| isTrivialForMSVC(RD, FI.getReturnType(), CGM); | ||
|
|
||
| // MSVC always returns structs indirectly from C++ instance methods. | ||
| isIndirectReturn = !isTrivialForABI || FI.isInstanceMethod(); | ||
| } else if (isa<VectorType>(FI.getReturnType())) { | ||
| // On x86, MSVC seems to only return vector types indirectly from non- | ||
| // vectorcall C++ instance methods. | ||
| isIndirectReturn = | ||
| CGM.getTarget().getTriple().isX86() && FI.isInstanceMethod() && | ||
|
||
| FI.getCallingConvention() != llvm::CallingConv::X86_VectorCall && | ||
| // Clang <= 21.0 did not do this. | ||
| getContext().getLangOpts().getClangABICompat() > | ||
| LangOptions::ClangABI::Ver21; | ||
| } | ||
|
|
||
| if (isIndirectReturn) { | ||
| CharUnits Align = CGM.getContext().getTypeAlignInChars(FI.getReturnType()); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| // RUN: %clang_cc1 -ffreestanding -emit-llvm %s -o - -triple=i686-pc-windows-msvc \ | ||
| // RUN: | FileCheck --check-prefixes=CHECK,X86 %s | ||
| // RUN: %clang_cc1 -ffreestanding -emit-llvm %s -o - -triple=x86_64-pc-windows-msvc \ | ||
| // RUN: | FileCheck --check-prefixes=CHECK,X86 %s | ||
| // RUN: %clang_cc1 -ffreestanding -emit-llvm %s -o - -triple=aarch64-pc-windows-msvc \ | ||
| // RUN: | FileCheck --check-prefixes=CHECK,AARCH64 %s | ||
| // RUN: %clang_cc1 -ffreestanding -emit-llvm %s -o - -triple=i686-pc-windows-msvc \ | ||
| // RUN: -fclang-abi-compat=21 | FileCheck --check-prefixes=CHECK,CLANG21 %s | ||
| // RUN: %clang_cc1 -ffreestanding -emit-llvm %s -o - -triple=x86_64-pc-windows-msvc \ | ||
| // RUN: -fclang-abi-compat=21 | FileCheck --check-prefixes=CHECK,CLANG21 %s | ||
| // RUN: %clang_cc1 -ffreestanding -emit-llvm %s -o - -triple=aarch64-pc-windows-msvc \ | ||
| // RUN: -fclang-abi-compat=21 | FileCheck --check-prefixes=CHECK,CLANG21 %s | ||
|
|
||
| // To match the MSVC ABI, vector types must be returned indirectly from member | ||
| // functions on x86 and x86-64 (as long as they do not use the vectorcall | ||
| // calling convention), but must be returned directly everywhere else. | ||
|
|
||
| #if defined(__i386__) || defined(__x86_64__) | ||
| #include <xmmintrin.h> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please extend this test to cover arm64. I fed it to an AI tool and it did the straightforward rewrite, which I think you can adapt with some ifdefs to make this a single test case: This demonstrates that ARM64 passes vectors directly. Honestly, I found that surprising, because if you look into arm(64)_neon.h, you see that MSVC defines vectors as "intrinsic_type" structs, so it makes sense that "records" go down the path of "instance methods return structs indirectly". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. (I was also surprised by MSVC returning vectors directly on ARM64, but that's also why I constrained this behavior to only apply to x86/x86-64 targets.) |
||
| #define VECTOR_TYPE __m128 | ||
| #endif | ||
|
|
||
| #ifdef __aarch64__ | ||
| #include <arm_neon.h> | ||
| #define VECTOR_TYPE float32x4_t | ||
| #endif | ||
|
|
||
| struct Foo { | ||
| VECTOR_TYPE method_ret_vec(); | ||
| VECTOR_TYPE __vectorcall vectorcall_method_ret_vec(); | ||
| }; | ||
|
|
||
| VECTOR_TYPE Foo::method_ret_vec() { | ||
| return VECTOR_TYPE{}; | ||
| // GH104 | ||
| // X86: store <4 x float> | ||
| // X86: ret void | ||
| // AARCH64: ret <4 x float> | ||
| // CLANG21: ret <4 x float> | ||
| } | ||
|
|
||
| VECTOR_TYPE __vectorcall Foo::vectorcall_method_ret_vec() { | ||
| return VECTOR_TYPE{}; | ||
| // CHECK: ret <4 x float> | ||
| } | ||
|
|
||
| VECTOR_TYPE func_ret_vec() { | ||
| return VECTOR_TYPE{}; | ||
| // CHECK: ret <4 x float> | ||
| } | ||
|
|
||
| VECTOR_TYPE __vectorcall vectorcall_func_ret_vec() { | ||
| return VECTOR_TYPE{}; | ||
| // CHECK: ret <4 x float> | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.