Skip to content
Open
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ C++ Specific Potentially Breaking Changes
ABI Changes in This Version
---------------------------

- Fixed Microsoft calling convention for returning vector types from C++ member
functions. Such vector types should be returned indirectly. (GH#104)

AST Dumping Potentially Breaking Changes
----------------------------------------
- How nested name specifiers are dumped and printed changes, keeping track of clang AST changes.
Expand Down
23 changes: 14 additions & 9 deletions clang/lib/CodeGen/MicrosoftCXXABI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1168,15 +1168,20 @@ 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() &&
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is worth expanding this out into if blocks. I think you can put the ABI ver check into the else if condition above, and combine the vector call check with a size check for > 64 bits to be ABI compatible with __vectorcall __m64. Please add a test for that as well.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

FI.getCallingConvention() != llvm::CallingConv::X86_VectorCall;
}

if (isIndirectReturn) {
CharUnits Align = CGM.getContext().getTypeAlignInChars(FI.getReturnType());
Expand Down
36 changes: 36 additions & 0 deletions clang/test/CodeGenCXX/microsoft-abi-vector-types.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// RUN: %clang_cc1 -ffreestanding -emit-llvm %s -o - -triple=i686-pc-windows-msvc | FileCheck %s
// RUN: %clang_cc1 -ffreestanding -emit-llvm %s -o - -triple=x86_64-pc-windows-msvc | FileCheck %s

// To match the MSVC ABI, vector types must be returned indirectly from member
// functions (as long as they do not use the vectorcall calling convention),
// but must be returned directly everywhere else.

#include <xmmintrin.h>
Copy link
Collaborator

Choose a reason for hiding this comment

The 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:
https://godbolt.org/z/xWon6163j

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".

Copy link
Author

Choose a reason for hiding this comment

The 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.)


struct Foo {
__m128 method_m128();
__m128 __vectorcall vectorcall_method_m128();
};

__m128 Foo::method_m128() {
return __m128{};
// GH104
// CHECK: store <4 x float>
// CHECK: ret void
}

__m128 __vectorcall Foo::vectorcall_method_m128() {
return __m128{};
// CHECK: ret <4 x float>
}

__m128 func_m128() {
return __m128{};
// CHECK: ret <4 x float>
}

__m128 __vectorcall vectorcall_func_m128() {
return __m128{};
// CHECK: ret <4 x float>
}