[clang] 5fbab40 - [MS] Apply `inreg` to AArch64 sret parms on instance methods
Reid Kleckner via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 15 15:00:09 PDT 2020
Author: Reid Kleckner
Date: 2020-10-15T14:54:42-07:00
New Revision: 5fbab4025eb57b12f2842ab188ff07a110708e1d
URL: https://github.com/llvm/llvm-project/commit/5fbab4025eb57b12f2842ab188ff07a110708e1d
DIFF: https://github.com/llvm/llvm-project/commit/5fbab4025eb57b12f2842ab188ff07a110708e1d.diff
LOG: [MS] Apply `inreg` to AArch64 sret parms on instance methods
The documentation rules indicate that instance methods should return
large, trivially copyable aggregates via X1/X0 and not X8 as is normally
done when returning such structs from free functions:
https://docs.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions?view=vs-2019#return-values
Fixes PR47836, a bug in the initial implementation of these rules.
I tried to simplify the logic a bit as well while I'm here.
Differential Revision: https://reviews.llvm.org/D89362
Added:
Modified:
clang/lib/CodeGen/MicrosoftCXXABI.cpp
clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
index cae79099dd6b..31e7e4c66901 100644
--- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1070,11 +1070,7 @@ bool MicrosoftCXXABI::hasMostDerivedReturn(GlobalDecl GD) const {
return isDeletingDtor(GD);
}
-static bool IsSizeGreaterThan128(const CXXRecordDecl *RD) {
- return RD->getASTContext().getTypeSize(RD->getTypeForDecl()) > 128;
-}
-
-static bool hasMicrosoftABIRestrictions(const CXXRecordDecl *RD) {
+static bool isCXX14Aggregate(const CXXRecordDecl *RD) {
// For AArch64, we use the C++14 definition of an aggregate, so we also
// check for:
// No private or protected non static data members.
@@ -1083,19 +1079,19 @@ static bool hasMicrosoftABIRestrictions(const CXXRecordDecl *RD) {
// Additionally, we need to ensure that there is a trivial copy assignment
// operator, a trivial destructor and no user-provided constructors.
if (RD->hasProtectedFields() || RD->hasPrivateFields())
- return true;
+ return false;
if (RD->getNumBases() > 0)
- return true;
+ return false;
if (RD->isPolymorphic())
- return true;
+ return false;
if (RD->hasNonTrivialCopyAssignment())
- return true;
+ return false;
for (const CXXConstructorDecl *Ctor : RD->ctors())
if (Ctor->isUserProvided())
- return true;
+ return false;
if (RD->hasNonTrivialDestructor())
- return true;
- return false;
+ return false;
+ return true;
}
bool MicrosoftCXXABI::classifyReturnType(CGFunctionInfo &FI) const {
@@ -1103,21 +1099,29 @@ bool MicrosoftCXXABI::classifyReturnType(CGFunctionInfo &FI) const {
if (!RD)
return false;
+ // Normally, the C++ concept of "is trivially copyable" is used to determine
+ // if a struct can be returned directly. However, as MSVC and the language
+ // have evolved, the definition of "trivially copyable" has changed, while the
+ // ABI must remain stable. AArch64 uses the C++14 concept of an "aggregate",
+ // while other ISAs use the older concept of "plain old data".
+ bool isTrivialForABI = RD->isPOD();
bool isAArch64 = CGM.getTarget().getTriple().isAArch64();
- bool isSimple = !isAArch64 || !hasMicrosoftABIRestrictions(RD);
- bool isIndirectReturn =
- isAArch64 ? (!RD->canPassInRegisters() ||
- IsSizeGreaterThan128(RD))
- : !RD->isPOD();
- bool isInstanceMethod = FI.isInstanceMethod();
-
- if (isIndirectReturn || !isSimple || isInstanceMethod) {
+ if (isAArch64)
+ isTrivialForABI = RD->canPassInRegisters() && isCXX14Aggregate(RD);
+
+ // MSVC always returns structs indirectly from C++ instance methods.
+ bool isIndirectReturn = !isTrivialForABI || FI.isInstanceMethod();
+
+ if (isIndirectReturn) {
CharUnits Align = CGM.getContext().getTypeAlignInChars(FI.getReturnType());
FI.getReturnInfo() = ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
- FI.getReturnInfo().setSRetAfterThis(isInstanceMethod);
- FI.getReturnInfo().setInReg(isAArch64 &&
- !(isSimple && IsSizeGreaterThan128(RD)));
+ // MSVC always passes `this` before the `sret` parameter.
+ FI.getReturnInfo().setSRetAfterThis(FI.isInstanceMethod());
+
+ // On AArch64, use the `inreg` attribute if the object is considered to not
+ // be trivially copyable, or if this is an instance method struct return.
+ FI.getReturnInfo().setInReg(isAArch64);
return true;
}
diff --git a/clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp b/clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
index c490deea4c28..139fd88e6e09 100644
--- a/clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
+++ b/clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
@@ -87,17 +87,20 @@ Small small_return() { return Small(); }
// LINUX-LABEL: define void @_Z12small_returnv(%struct.Small* noalias sret align 4 %agg.result)
// WIN32: define dso_local i32 @"?small_return@@YA?AUSmall@@XZ"()
// WIN64: define dso_local i32 @"?small_return@@YA?AUSmall@@XZ"()
+// WOA64: define dso_local i64 @"?small_return@@YA?AUSmall@@XZ"()
Medium medium_return() { return Medium(); }
// LINUX-LABEL: define void @_Z13medium_returnv(%struct.Medium* noalias sret align 4 %agg.result)
// WIN32: define dso_local i64 @"?medium_return@@YA?AUMedium@@XZ"()
// WIN64: define dso_local i64 @"?medium_return@@YA?AUMedium@@XZ"()
+// WOA64: define dso_local i64 @"?medium_return@@YA?AUMedium@@XZ"()
// Returning structs that fit into a register but are not POD.
SmallCpp11NotCpp03Pod small_non_pod_return() { return SmallCpp11NotCpp03Pod(); }
// LINUX-LABEL: define void @_Z20small_non_pod_returnv(%struct.SmallCpp11NotCpp03Pod* noalias sret align 4 %agg.result)
// WIN32: define dso_local void @"?small_non_pod_return@@YA?AUSmallCpp11NotCpp03Pod@@XZ"(%struct.SmallCpp11NotCpp03Pod* noalias sret align 4 %agg.result)
// WIN64: define dso_local void @"?small_non_pod_return@@YA?AUSmallCpp11NotCpp03Pod@@XZ"(%struct.SmallCpp11NotCpp03Pod* noalias sret align 4 %agg.result)
+// WOA64: define dso_local void @"?small_non_pod_return@@YA?AUSmallCpp11NotCpp03Pod@@XZ"(%struct.SmallCpp11NotCpp03Pod* inreg noalias sret align 4 %agg.result)
SmallWithCtor small_with_ctor_return() { return SmallWithCtor(); }
// LINUX-LABEL: define void @_Z22small_with_ctor_returnv(%struct.SmallWithCtor* noalias sret align 4 %agg.result)
@@ -106,23 +109,33 @@ SmallWithCtor small_with_ctor_return() { return SmallWithCtor(); }
// FIXME: The 'sret' mark here doesn't seem to be enough to convince LLVM to
// preserve the hidden sret pointer in R0 across the function.
// WOA: define dso_local arm_aapcs_vfpcc void @"?small_with_ctor_return@@YA?AUSmallWithCtor@@XZ"(%struct.SmallWithCtor* noalias sret align 4 %agg.result)
+// WOA64: define dso_local void @"?small_with_ctor_return@@YA?AUSmallWithCtor@@XZ"(%struct.SmallWithCtor* inreg noalias sret align 4 %agg.result)
+
+SmallWithDtor small_with_dtor_return() { return SmallWithDtor(); }
+// LINUX-LABEL: define void @_Z22small_with_dtor_returnv(%struct.SmallWithDtor* noalias sret align 4 %agg.result)
+// WIN32: define dso_local void @"?small_with_dtor_return@@YA?AUSmallWithDtor@@XZ"(%struct.SmallWithDtor* noalias sret align 4 %agg.result)
+// WIN64: define dso_local void @"?small_with_dtor_return@@YA?AUSmallWithDtor@@XZ"(%struct.SmallWithDtor* noalias sret align 4 %agg.result)
+// WOA64: define dso_local void @"?small_with_dtor_return@@YA?AUSmallWithDtor@@XZ"(%struct.SmallWithDtor* inreg noalias sret align 4 %agg.result)
SmallWithVftable small_with_vftable_return() { return SmallWithVftable(); }
// LINUX-LABEL: define void @_Z25small_with_vftable_returnv(%struct.SmallWithVftable* noalias sret align 4 %agg.result)
// WIN32: define dso_local void @"?small_with_vftable_return@@YA?AUSmallWithVftable@@XZ"(%struct.SmallWithVftable* noalias sret align 4 %agg.result)
// WIN64: define dso_local void @"?small_with_vftable_return@@YA?AUSmallWithVftable@@XZ"(%struct.SmallWithVftable* noalias sret align 8 %agg.result)
+// WOA64: define dso_local void @"?small_with_vftable_return@@YA?AUSmallWithVftable@@XZ"(%struct.SmallWithVftable* inreg noalias sret align 8 %agg.result)
MediumWithCopyCtor medium_with_copy_ctor_return() { return MediumWithCopyCtor(); }
// LINUX-LABEL: define void @_Z28medium_with_copy_ctor_returnv(%struct.MediumWithCopyCtor* noalias sret align 4 %agg.result)
// WIN32: define dso_local void @"?medium_with_copy_ctor_return@@YA?AUMediumWithCopyCtor@@XZ"(%struct.MediumWithCopyCtor* noalias sret align 4 %agg.result)
// WIN64: define dso_local void @"?medium_with_copy_ctor_return@@YA?AUMediumWithCopyCtor@@XZ"(%struct.MediumWithCopyCtor* noalias sret align 4 %agg.result)
// WOA: define dso_local arm_aapcs_vfpcc void @"?medium_with_copy_ctor_return@@YA?AUMediumWithCopyCtor@@XZ"(%struct.MediumWithCopyCtor* noalias sret align 4 %agg.result)
+// WOA64: define dso_local void @"?medium_with_copy_ctor_return@@YA?AUMediumWithCopyCtor@@XZ"(%struct.MediumWithCopyCtor* inreg noalias sret align 4 %agg.result)
// Returning a large struct that doesn't fit into a register.
Big big_return() { return Big(); }
// LINUX-LABEL: define void @_Z10big_returnv(%struct.Big* noalias sret align 4 %agg.result)
// WIN32: define dso_local void @"?big_return@@YA?AUBig@@XZ"(%struct.Big* noalias sret align 4 %agg.result)
// WIN64: define dso_local void @"?big_return@@YA?AUBig@@XZ"(%struct.Big* noalias sret align 4 %agg.result)
+// WOA64: define dso_local void @"?big_return@@YA?AUBig@@XZ"(%struct.Big* noalias sret align 4 %agg.result)
void small_arg(Small s) {}
@@ -284,11 +297,13 @@ class Class {
// LINUX: define {{.*}} void @_ZN5Class21thiscall_method_smallEv(%struct.Small* noalias sret align 4 %agg.result, %class.Class* %this)
// WIN32: define {{.*}} x86_thiscallcc void @"?thiscall_method_small at Class@@QAE?AUSmall@@XZ"(%class.Class* %this, %struct.Small* noalias sret align 4 %agg.result)
// WIN64: define linkonce_odr dso_local void @"?thiscall_method_small at Class@@QEAA?AUSmall@@XZ"(%class.Class* %this, %struct.Small* noalias sret align 4 %agg.result)
+ // WOA64: define linkonce_odr dso_local void @"?thiscall_method_small at Class@@QEAA?AUSmall@@XZ"(%class.Class* %this, %struct.Small* inreg noalias sret align 4 %agg.result)
SmallWithCtor thiscall_method_small_with_ctor() { return SmallWithCtor(); }
// LINUX: define {{.*}} void @_ZN5Class31thiscall_method_small_with_ctorEv(%struct.SmallWithCtor* noalias sret align 4 %agg.result, %class.Class* %this)
// WIN32: define {{.*}} x86_thiscallcc void @"?thiscall_method_small_with_ctor at Class@@QAE?AUSmallWithCtor@@XZ"(%class.Class* %this, %struct.SmallWithCtor* noalias sret align 4 %agg.result)
// WIN64: define linkonce_odr dso_local void @"?thiscall_method_small_with_ctor at Class@@QEAA?AUSmallWithCtor@@XZ"(%class.Class* %this, %struct.SmallWithCtor* noalias sret align 4 %agg.result)
+ // WOA64: define linkonce_odr dso_local void @"?thiscall_method_small_with_ctor at Class@@QEAA?AUSmallWithCtor@@XZ"(%class.Class* %this, %struct.SmallWithCtor* inreg noalias sret align 4 %agg.result)
Small __cdecl cdecl_method_small() { return Small(); }
// LINUX: define {{.*}} void @_ZN5Class18cdecl_method_smallEv(%struct.Small* noalias sret align 4 %agg.result, %class.Class* %this)
@@ -299,6 +314,7 @@ class Class {
// LINUX: define {{.*}} void @_ZN5Class16cdecl_method_bigEv(%struct.Big* noalias sret align 4 %agg.result, %class.Class* %this)
// WIN32: define {{.*}} void @"?cdecl_method_big at Class@@QAA?AUBig@@XZ"(%class.Class* %this, %struct.Big* noalias sret align 4 %agg.result)
// WIN64: define linkonce_odr dso_local void @"?cdecl_method_big at Class@@QEAA?AUBig@@XZ"(%class.Class* %this, %struct.Big* noalias sret align 4 %agg.result)
+ // WOA64: define linkonce_odr dso_local void @"?cdecl_method_big at Class@@QEAA?AUBig@@XZ"(%class.Class* %this, %struct.Big* inreg noalias sret align 4 %agg.result)
void thiscall_method_arg(Empty s) {}
// LINUX: define {{.*}} void @_ZN5Class19thiscall_method_argE5Empty(%class.Class* %this)
More information about the cfe-commits
mailing list