[COFF, ARM64] Fix ABI implementation of struct returns

Summary:
Related llvm patch: D60348.
Patch co-authored by Sanjin Sijaric.

Reviewers: rnk, efriedma, TomTan, ssijaric, ostannard

Reviewed By: efriedma

Subscribers: dmajor, richard.townsend.arm, ostannard, javed.absar, kristof.beyls, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D60349

llvm-svn: 359932

clang/include/clang/AST/DeclCXX.h

@@ -1325,6 +1325,14 @@ public:
   /// \note This does NOT include a check for union-ness.
   bool isEmpty() const { return data().Empty; }
 
+  bool hasPrivateFields() const {
+    return data().HasPrivateFields;
+  }
+
+  bool hasProtectedFields() const {
+    return data().HasProtectedFields;
+  }
+
   /// Determine whether this class has direct non-static data members.
   bool hasDirectFields() const {
     auto &D = data();

clang/include/clang/CodeGen/CGFunctionInfo.h

@@ -95,7 +95,6 @@ private:
   bool InReg : 1;           // isDirect() || isExtend() || isIndirect()
   bool CanBeFlattened: 1;   // isDirect()
   bool SignExt : 1;         // isExtend()
-  bool SuppressSRet : 1;    // isIndirect()
 
   bool canHavePaddingType() const {
     return isDirect() || isExtend() || isIndirect() || isExpand();
@@ -111,14 +110,13 @@ private:
   }
 
   ABIArgInfo(Kind K)
-      : TheKind(K), PaddingInReg(false), InReg(false), SuppressSRet(false) {
+      : TheKind(K), PaddingInReg(false), InReg(false) {
   }
 
 public:
   ABIArgInfo()
       : TypeData(nullptr), PaddingType(nullptr), DirectOffset(0),
-        TheKind(Direct), PaddingInReg(false), InReg(false),
-        SuppressSRet(false) {}
+        TheKind(Direct), PaddingInReg(false), InReg(false) {}
 
   static ABIArgInfo getDirect(llvm::Type *T = nullptr, unsigned Offset = 0,
                               llvm::Type *Padding = nullptr,
@@ -407,16 +405,6 @@ public:
     CanBeFlattened = Flatten;
   }
 
-  bool getSuppressSRet() const {
-    assert(isIndirect() && "Invalid kind!");
-    return SuppressSRet;
-  }
-
-  void setSuppressSRet(bool Suppress) {
-    assert(isIndirect() && "Invalid kind!");
-    SuppressSRet = Suppress;
-  }
-
   void dump() const;
 };
 

clang/lib/CodeGen/CGCall.cpp

@@ -1999,8 +1999,7 @@ void CodeGenModule::ConstructAttributeList(
   // Attach attributes to sret.
   if (IRFunctionArgs.hasSRetArg()) {
     llvm::AttrBuilder SRETAttrs;
-    if (!RetAI.getSuppressSRet())
-      SRETAttrs.addAttribute(llvm::Attribute::StructRet);
+    SRETAttrs.addAttribute(llvm::Attribute::StructRet);
     hasUsedSRet = true;
     if (RetAI.getInReg())
       SRETAttrs.addAttribute(llvm::Attribute::InReg);

clang/lib/CodeGen/MicrosoftCXXABI.cpp

@@ -1051,33 +1051,55 @@ 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) {
+  // For AArch64, we use the C++14 definition of an aggregate, so we also
+  // check for:
+  //   No private or protected non static data members.
+  //   No base classes
+  //   No virtual functions
+  // 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;
+  if (RD->getNumBases() > 0)
+    return true;
+  if (RD->isPolymorphic())
+    return true;
+  if (RD->hasNonTrivialCopyAssignment())
+    return true;
+  for (const CXXConstructorDecl *Ctor : RD->ctors())
+    if (Ctor->isUserProvided())
+      return true;
+  if (RD->hasNonTrivialDestructor())
+    return true;
+  return false;
+}
+
 bool MicrosoftCXXABI::classifyReturnType(CGFunctionInfo &FI) const {
   const CXXRecordDecl *RD = FI.getReturnType()->getAsCXXRecordDecl();
   if (!RD)
     return false;
 
-  CharUnits Align = CGM.getContext().getTypeAlignInChars(FI.getReturnType());
-  if (FI.isInstanceMethod()) {
-    // If it's an instance method, aggregates are always returned indirectly via
-    // the second parameter.
-    FI.getReturnInfo() = ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
-    FI.getReturnInfo().setSRetAfterThis(FI.isInstanceMethod());
+  bool isAArch64 = CGM.getTarget().getTriple().isAArch64();
+  bool isSimple = !isAArch64 || !hasMicrosoftABIRestrictions(RD);
+  bool isIndirectReturn =
+      isAArch64 ? (!RD->canPassInRegisters() ||
+                   IsSizeGreaterThan128(RD))
+                : !RD->isPOD();
+  bool isInstanceMethod = FI.isInstanceMethod();
 
-    // aarch64-windows requires that instance methods use X1 for the return
-    // address. So for aarch64-windows we do not mark the
-    // return as SRet.
-    FI.getReturnInfo().setSuppressSRet(CGM.getTarget().getTriple().getArch() ==
-                                       llvm::Triple::aarch64);
-    return true;
-  } else if (!RD->isPOD()) {
-    // If it's a free function, non-POD types are returned indirectly.
+  if (isIndirectReturn || !isSimple || isInstanceMethod) {
+    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)));
 
-    // aarch64-windows requires that non-POD, non-instance returns use X0 for
-    // the return address. So for aarch64-windows we do not mark the return as
-    // SRet.
-    FI.getReturnInfo().setSuppressSRet(CGM.getTarget().getTriple().getArch() ==
-                                       llvm::Triple::aarch64);
     return true;
   }
 

clang/lib/Sema/SemaDeclCXX.cpp

@@ -5957,8 +5957,11 @@ static bool canPassInRegisters(Sema &S, CXXRecordDecl *D,
 
     // Note: This permits small classes with nontrivial destructors to be
     // passed in registers, which is non-conforming.
+    bool isAArch64 = S.Context.getTargetInfo().getTriple().isAArch64();
+    uint64_t TypeSize = isAArch64 ? 128 : 64;
+
     if (CopyCtorIsTrivial &&
-        S.getASTContext().getTypeSize(D->getTypeForDecl()) <= 64)
+        S.getASTContext().getTypeSize(D->getTypeForDecl()) <= TypeSize)
       return true;
     return false;
   }

clang/test/CodeGen/arm64-microsoft-arguments.cpp

@@ -1,25 +1,203 @@
 // RUN: %clang_cc1 -triple aarch64-windows -ffreestanding -emit-llvm -O0 \
 // RUN: -x c++ -o - %s | FileCheck %s
 
-struct pod { int a, b, c, d, e; };
-
-struct non_pod {
-  int a;
-  non_pod() {}
+// Pass and return for type size <= 8 bytes.
+// CHECK: define {{.*}} i64 @{{.*}}f1{{.*}}()
+// CHECK: call i64 {{.*}}func1{{.*}}(i64 %3)
+struct S1 {
+  int a[2];
 };
 
-struct pod s;
-struct non_pod t;
+S1 func1(S1 x);
+S1 f1() {
+  S1 x;
+  return func1(x);
+}
 
-struct pod bar() { return s; }
-struct non_pod foo() { return t; }
-// CHECK: define {{.*}} void @{{.*}}bar{{.*}}(%struct.pod* noalias sret %agg.result)
-// CHECK: define {{.*}} void @{{.*}}foo{{.*}}(%struct.non_pod* noalias %agg.result)
+// Pass and return type size <= 16 bytes.
+// CHECK: define {{.*}} [2 x i64] @{{.*}}f2{{.*}}()
+// CHECK: call [2 x i64] {{.*}}func2{{.*}}([2 x i64] %3)
+struct S2 {
+  int a[4];
+};
+
+S2 func2(S2 x);
+S2 f2() {
+  S2 x;
+  return func2(x);
+}
+
+// Pass and return for type size > 16 bytes.
+// CHECK: define {{.*}} void @{{.*}}f3{{.*}}(%struct.S3* noalias sret %agg.result)
+// CHECK: call void {{.*}}func3{{.*}}(%struct.S3* sret %agg.result, %struct.S3* %agg.tmp)
+struct S3 {
+  int a[5];
+};
+
+S3 func3(S3 x);
+S3 f3() {
+  S3 x;
+  return func3(x);
+}
+
+// Pass and return aggregate (of size < 16 bytes) with non-trivial destructor.
+// Passed directly but returned indirectly.
+// CHECK: define {{.*}} void {{.*}}f4{{.*}}(%struct.S4* inreg noalias sret %agg.result)
+// CHECK: call void {{.*}}func4{{.*}}(%struct.S4* inreg sret %agg.result, [2 x i64] %4)
+struct S4 {
+  int a[3];
+  ~S4();
+};
+
+S4 func4(S4 x);
+S4 f4() {
+  S4 x;
+  return func4(x);
+}
+
+// Pass and return from instance method called from instance method.
+// CHECK: define {{.*}} void @{{.*}}bar@Q1{{.*}}(%class.Q1* %this, %class.P1* inreg noalias sret %agg.result)
+// CHECK: call void {{.*}}foo@P1{{.*}}(%class.P1* %ref.tmp, %class.P1* inreg sret %agg.result, i8 %0)
+
+class P1 {
+public:
+  P1 foo(P1 x);
+};
+
+class Q1 {
+public:
+  P1 bar();
+};
+
+P1 Q1::bar() {
+  P1 p1;
+  return P1().foo(p1);
+}
+
+// Pass and return from instance method called from free function.
+// CHECK: define {{.*}} void {{.*}}bar{{.*}}()
+// CHECK: call void {{.*}}foo@P2{{.*}}(%class.P2* %ref.tmp, %class.P2* inreg sret %retval, i8 %0)
+class P2 {
+public:
+  P2 foo(P2 x);
+};
+
+P2 bar() {
+  P2 p2;
+  return P2().foo(p2);
+}
+
+// Pass and return an object with a user-provided constructor (passed directly,
+// returned indirectly)
+// CHECK: define {{.*}} void @{{.*}}f5{{.*}}(%struct.S5* inreg noalias sret %agg.result)
+// CHECK: call void {{.*}}func5{{.*}}(%struct.S5* inreg sret %agg.result, i64 {{.*}})
+struct S5 {
+  S5();
+  int x;
+};
+
+S5 func5(S5 x);
+S5 f5() {
+  S5 x;
+  return func5(x);
+}
+
+// Pass and return an object with a non-trivial explicitly defaulted constructor
+// (passed directly, returned directly)
+// CHECK: define {{.*}} i64 @"?f6@@YA?AUS6@@XZ"()
+// CHECK: call i64 {{.*}}func6{{.*}}(i64 {{.*}})
+struct S6a {
+  S6a();
+};
+
+struct S6 {
+  S6() = default;
+  S6a x;
+};
+
+S6 func6(S6 x);
+S6 f6() {
+  S6 x;
+  return func6(x);
+}
+
+// Pass and return an object with a non-trivial implicitly defaulted constructor
+// (passed directly, returned directly)
+// CHECK: define {{.*}} i64 @"?f7@@YA?AUS7@@XZ"()
+// CHECK: call i64 {{.*}}func7{{.*}}(i64 {{.*}})
+struct S7 {
+  S6a x;
+};
+
+S7 func7(S7 x);
+S7 f7() {
+  S7 x;
+  return func7(x);
+}
+
+struct S8a {
+  ~S8a();
+};
+
+// Pass and return an object with a non-trivial default destructor (passed
+// directly, returne indirectly)
+struct S8 {
+  S8a x;
+  int y;
+};
+
+// CHECK: define {{.*}} void {{.*}}?f8{{.*}}(%struct.S8* inreg noalias sret {{.*}})
+// CHECK: call void {{.*}}func8{{.*}}(%struct.S8* inreg sret {{.*}}, i64 {{.*}})
+S8 func8(S8 x);
+S8 f8() {
+  S8 x;
+  return func8(x);
+}
 
 
-// Check instance methods.
-struct pod2 { int x; };
-struct Baz { pod2 baz(); };
+// Pass and return an object with a non-trivial copy-assignment operator and
+// a trivial copy constructor (passed directly, returned indirectly)
+// CHECK: define {{.*}} void @"?f9@@YA?AUS9@@XZ"(%struct.S9* inreg noalias sret {{.*}})
+// CHECK: call void {{.*}}func9{{.*}}(%struct.S9* inreg sret {{.*}}, i64 {{.*}})
+struct S9 {
+  S9& operator=(const S9&);
+  int x;
+};
 
-int qux() { return Baz().baz().x; }
-// CHECK: declare {{.*}} void @{{.*}}baz@Baz{{.*}}(%struct.Baz*, %struct.pod2*)
+S9 func9(S9 x);
+S9 f9() {
+  S9 x;
+  S9 y = x;
+  x = y;
+  return func9(x);
+}
+
+// Pass and return an object with a base class (passed directly, returned
+// indirectly).
+// CHECK: define dso_local void {{.*}}f10{{.*}}(%struct.S10* inreg noalias sret {{.*}})
+// CHECK: call void {{.*}}func10{{.*}}(%struct.S10* inreg sret {{.*}}, [2 x i64] {{.*}})
+struct S10 : public S1 {
+  int x;
+};
+
+S10 func10(S10 x);
+S10 f10() {
+  S10 x;
+  return func10(x);
+}
+
+
+// Pass and return a non aggregate object exceeding > 128 bits (passed
+// indirectly, returned indirectly)
+// CHECK: define dso_local void {{.*}}f11{{.*}}(%struct.S11* inreg noalias sret {{.*}})
+// CHECK: call void {{.*}}func11{{.*}}(%struct.S11* inreg sret {{.*}}, %struct.S11* {{.*}})
+struct S11 {
+  virtual void f();
+  int a[5];
+};
+
+S11 func11(S11 x);
+S11 f11() {
+  S11 x;
+  return func11(x);
+}

clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp

@@ -69,6 +69,11 @@ struct BaseNoByval : Small {
   int bb;
 };
 
+struct SmallWithPrivate {
+private:
+ int i;
+};
+
 // WIN32: declare dso_local void @"{{.*take_bools_and_chars.*}}"
 // WIN32:       (<{ i8, [3 x i8], i8, [3 x i8], %struct.SmallWithDtor,
 // WIN32:           i8, [3 x i8], i8, [3 x i8], i32, i8, [3 x i8] }>* inalloca)
@@ -165,7 +170,7 @@ void small_arg_with_dtor(SmallWithDtor s) {}
 // WIN64:   call void @"??1SmallWithDtor@@QEAA@XZ"
 // WIN64: }
 // WOA64: define dso_local void @"?small_arg_with_dtor@@YAXUSmallWithDtor@@@Z"(i64 %s.coerce) {{.*}} {
-// WOA64:   call void @"??1SmallWithDtor@@QEAA@XZ"
+// WOA64:   call void @"??1SmallWithDtor@@QEAA@XZ"(%struct.SmallWithDtor* %s)
 // WOA64: }
 
 // FIXME: MSVC incompatible!
@@ -173,6 +178,12 @@ void small_arg_with_dtor(SmallWithDtor s) {}
 // WOA:   call arm_aapcs_vfpcc void @"??1SmallWithDtor@@QAA@XZ"(%struct.SmallWithDtor* %s)
 // WOA: }
 
+
+// Test that the eligible non-aggregate is passed directly, but returned
+// indirectly on ARM64 Windows.
+// WOA64: define dso_local void @"?small_arg_with_private_member@@YA?AUSmallWithPrivate@@U1@@Z"(%struct.SmallWithPrivate* inreg noalias sret %agg.result, i64 %s.coerce) {{.*}} {
+SmallWithPrivate small_arg_with_private_member(SmallWithPrivate s) { return s; }
+
 void call_small_arg_with_dtor() {
   small_arg_with_dtor(SmallWithDtor());
 }