[clang] e5f51fd - [clang][aarch64] Precondition isHomogeneousAggregate on isCXX14Aggregate

David Truby via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 12 11:44:14 PST 2021


Author: David Truby
Date: 2021-01-12T19:44:01Z
New Revision: e5f51fdd650c6d20c81fedb8e856e9858aa10991

URL: https://github.com/llvm/llvm-project/commit/e5f51fdd650c6d20c81fedb8e856e9858aa10991
DIFF: https://github.com/llvm/llvm-project/commit/e5f51fdd650c6d20c81fedb8e856e9858aa10991.diff

LOG: [clang][aarch64] Precondition isHomogeneousAggregate on isCXX14Aggregate

MSVC on WoA64 includes isCXX14Aggregate in its definition. This is de-facto
specification on that platform, so match msvc's behaviour.

Fixes: https://bugs.llvm.org/show_bug.cgi?id=47611

Co-authored-by: Peter Waller <peter.waller at arm.com>

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

Added: 
    

Modified: 
    clang/lib/CodeGen/CGCXXABI.h
    clang/lib/CodeGen/MicrosoftCXXABI.cpp
    clang/lib/CodeGen/TargetInfo.cpp
    clang/test/CodeGenCXX/homogeneous-aggregates.cpp
    llvm/test/CodeGen/AArch64/arm64-windows-calls.ll

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CGCXXABI.h b/clang/lib/CodeGen/CGCXXABI.h
index 171428a3525d..ea839db7528e 100644
--- a/clang/lib/CodeGen/CGCXXABI.h
+++ b/clang/lib/CodeGen/CGCXXABI.h
@@ -146,6 +146,13 @@ class CGCXXABI {
   /// 'this' parameter of C++ instance methods.
   virtual bool isSRetParameterAfterThis() const { return false; }
 
+  /// Returns true if the ABI permits the argument to be a homogeneous
+  /// aggregate.
+  virtual bool
+  isPermittedToBeHomogeneousAggregate(const CXXRecordDecl *RD) const {
+    return true;
+  };
+
   /// Find the LLVM type used to represent the given member pointer
   /// type.
   virtual llvm::Type *

diff  --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
index c16c72dc93d5..cb0dc1d5d717 100644
--- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -771,6 +771,9 @@ class MicrosoftCXXABI : public CGCXXABI {
   LoadVTablePtr(CodeGenFunction &CGF, Address This,
                 const CXXRecordDecl *RD) override;
 
+  virtual bool
+  isPermittedToBeHomogeneousAggregate(const CXXRecordDecl *RD) const override;
+
 private:
   typedef std::pair<const CXXRecordDecl *, CharUnits> VFTableIdTy;
   typedef llvm::DenseMap<VFTableIdTy, llvm::GlobalVariable *> VTablesMapTy;
@@ -1070,7 +1073,7 @@ bool MicrosoftCXXABI::hasMostDerivedReturn(GlobalDecl GD) const {
   return isDeletingDtor(GD);
 }
 
-static bool isCXX14Aggregate(const CXXRecordDecl *RD) {
+static bool isTrivialForAArch64MSVC(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.
@@ -1107,7 +1110,7 @@ bool MicrosoftCXXABI::classifyReturnType(CGFunctionInfo &FI) const {
   bool isTrivialForABI = RD->isPOD();
   bool isAArch64 = CGM.getTarget().getTriple().isAArch64();
   if (isAArch64)
-    isTrivialForABI = RD->canPassInRegisters() && isCXX14Aggregate(RD);
+    isTrivialForABI = RD->canPassInRegisters() && isTrivialForAArch64MSVC(RD);
 
   // MSVC always returns structs indirectly from C++ instance methods.
   bool isIndirectReturn = !isTrivialForABI || FI.isInstanceMethod();
@@ -4358,3 +4361,12 @@ MicrosoftCXXABI::LoadVTablePtr(CodeGenFunction &CGF, Address This,
       performBaseAdjustment(CGF, This, QualType(RD->getTypeForDecl(), 0));
   return {CGF.GetVTablePtr(This, CGM.Int8PtrTy, RD), RD};
 }
+
+bool MicrosoftCXXABI::isPermittedToBeHomogeneousAggregate(
+    const CXXRecordDecl *CXXRD) const {
+  // MSVC Windows on Arm64 considers a type not HFA if it is not an
+  // aggregate according to the C++14 spec. This is not consistent with the
+  // AAPCS64, but is defacto spec on that platform.
+  return !CGM.getTarget().getTriple().isAArch64() ||
+         isTrivialForAArch64MSVC(CXXRD);
+}

diff  --git a/clang/lib/CodeGen/TargetInfo.cpp b/clang/lib/CodeGen/TargetInfo.cpp
index d36c7344e284..9a11a0720f3c 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -5065,8 +5065,12 @@ bool ABIInfo::isHomogeneousAggregate(QualType Ty, const Type *&Base,
 
     Members = 0;
 
-    // If this is a C++ record, check the bases first.
+    // If this is a C++ record, check the properties of the record such as
+    // bases and ABI specific restrictions
     if (const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
+      if (!getCXXABI().isPermittedToBeHomogeneousAggregate(CXXRD))
+        return false;
+
       for (const auto &I : CXXRD->bases()) {
         // Ignore empty records.
         if (isEmptyRecord(getContext(), I.getType(), true))

diff  --git a/clang/test/CodeGenCXX/homogeneous-aggregates.cpp b/clang/test/CodeGenCXX/homogeneous-aggregates.cpp
index 2b3af4226407..0fa30b2663bf 100644
--- a/clang/test/CodeGenCXX/homogeneous-aggregates.cpp
+++ b/clang/test/CodeGenCXX/homogeneous-aggregates.cpp
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -mfloat-abi hard -triple armv7-unknown-linux-gnueabi -emit-llvm -o - %s | FileCheck %s --check-prefix=ARM32
 // RUN: %clang_cc1 -mfloat-abi hard -triple aarch64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s --check-prefix=ARM64
 // RUN: %clang_cc1 -mfloat-abi hard -triple x86_64-unknown-windows-gnu -emit-llvm -o - %s | FileCheck %s --check-prefix=X64
+// RUN: %clang_cc1 -mfloat-abi hard -triple aarch64-unknown-windows-msvc -emit-llvm -o - %s | FileCheck %s --check-prefix=WOA64
 
 #if defined(__x86_64__)
 #define CC __attribute__((vectorcall))
@@ -104,3 +105,71 @@ struct HVAWithEmptyBitField : Float1, Float2 {
 // ARM32: define{{.*}} arm_aapcs_vfpcc void @_Z19with_empty_bitfield20HVAWithEmptyBitField(%struct.HVAWithEmptyBitField %a.coerce)
 // X64: define dso_local x86_vectorcallcc void @"\01_Z19with_empty_bitfield20HVAWithEmptyBitField@@16"(%struct.HVAWithEmptyBitField inreg %a.coerce)
 void CC with_empty_bitfield(HVAWithEmptyBitField a) {}
+
+namespace pr47611 {
+// MSVC on Arm includes "isCXX14Aggregate" as part of its definition of
+// Homogeneous Floating-point Aggregate (HFA). Additionally, it has a 
diff erent
+// handling of C++14 aggregates, which can lead to confusion.
+
+// Pod is a trivial HFA.
+struct Pod {
+  double b[2];
+};
+// Not an aggregate according to C++14 spec => not HFA according to MSVC.
+struct NotCXX14Aggregate {
+  NotCXX14Aggregate();
+  Pod p;
+};
+// NotPod is a C++14 aggregate. But not HFA, because it contains
+// NotCXX14Aggregate (which itself is not HFA because it's not a C++14
+// aggregate).
+struct NotPod {
+  NotCXX14Aggregate x;
+};
+struct Empty {};
+// A class with a base is returned using the sret calling convetion by MSVC.
+struct HasEmptyBase : public Empty {
+  double b[2];
+};
+struct HasPodBase : public Pod {};
+// WOA64-LABEL: define dso_local %"struct.pr47611::Pod" @"?copy at pr47611@@YA?AUPod at 1@PEAU21@@Z"(%"struct.pr47611::Pod"* %x)
+Pod copy(Pod *x) { return *x; } // MSVC: ldp d0,d1,[x0], Clang: ldp d0,d1,[x0]
+// WOA64-LABEL: define dso_local void @"?copy at pr47611@@YA?AUNotCXX14Aggregate at 1@PEAU21@@Z"(%"struct.pr47611::NotCXX14Aggregate"* inreg noalias sret(%"struct.pr47611::NotCXX14Aggregate") align 8 %agg.result, %"struct.pr47611::NotCXX14Aggregate"* %x)
+NotCXX14Aggregate copy(NotCXX14Aggregate *x) { return *x; } // MSVC: stp x8,x9,[x0], Clang: str q0,[x0]
+// WOA64-LABEL: define dso_local [2 x i64] @"?copy at pr47611@@YA?AUNotPod at 1@PEAU21@@Z"(%"struct.pr47611::NotPod"* %x)
+NotPod copy(NotPod *x) { return *x; }
+// WOA64-LABEL: define dso_local void @"?copy at pr47611@@YA?AUHasEmptyBase at 1@PEAU21@@Z"(%"struct.pr47611::HasEmptyBase"* inreg noalias sret(%"struct.pr47611::HasEmptyBase") align 8 %agg.result, %"struct.pr47611::HasEmptyBase"* %x)
+HasEmptyBase copy(HasEmptyBase *x) { return *x; }
+// WOA64-LABEL: define dso_local void @"?copy at pr47611@@YA?AUHasPodBase at 1@PEAU21@@Z"(%"struct.pr47611::HasPodBase"* inreg noalias sret(%"struct.pr47611::HasPodBase") align 8 %agg.result, %"struct.pr47611::HasPodBase"* %x)
+HasPodBase copy(HasPodBase *x) { return *x; }
+
+void call_copy_pod(Pod *pod) {
+  *pod = copy(pod);
+  // WOA64-LABEL: define dso_local void @"?call_copy_pod at pr47611@@YAXPEAUPod at 1@@Z"
+  // WOA64: %{{.*}} = call %"struct.pr47611::Pod" @"?copy at pr47611@@YA?AUPod at 1@PEAU21@@Z"(%"struct.pr47611::Pod"* %{{.*}})
+}
+
+void call_copy_notcxx14aggregate(NotCXX14Aggregate *notcxx14aggregate) {
+  *notcxx14aggregate = copy(notcxx14aggregate);
+  // WOA64-LABEL: define dso_local void @"?call_copy_notcxx14aggregate at pr47611@@YAXPEAUNotCXX14Aggregate at 1@@Z"
+  // WOA64: call void @"?copy at pr47611@@YA?AUNotCXX14Aggregate at 1@PEAU21@@Z"(%"struct.pr47611::NotCXX14Aggregate"* inreg sret(%"struct.pr47611::NotCXX14Aggregate") align 8 %{{.*}}, %"struct.pr47611::NotCXX14Aggregate"* %{{.*}})
+}
+
+void call_copy_notpod(NotPod *notPod) {
+  *notPod = copy(notPod);
+  // WOA64-LABEL: define dso_local void @"?call_copy_notpod at pr47611@@YAXPEAUNotPod at 1@@Z"
+  // WOA64: %{{.*}} = call [2 x i64] @"?copy at pr47611@@YA?AUNotPod at 1@PEAU21@@Z"(%"struct.pr47611::NotPod"* %{{.*}})
+}
+
+void call_copy_hasemptybase(HasEmptyBase *hasEmptyBase) {
+  *hasEmptyBase = copy(hasEmptyBase);
+  // WOA64-LABEL: define dso_local void @"?call_copy_hasemptybase at pr47611@@YAXPEAUHasEmptyBase at 1@@Z"
+  // WOA64: call void @"?copy at pr47611@@YA?AUHasEmptyBase at 1@PEAU21@@Z"(%"struct.pr47611::HasEmptyBase"* inreg sret(%"struct.pr47611::HasEmptyBase") align 8 %{{.*}}, %"struct.pr47611::HasEmptyBase"* %{{.*}})
+}
+
+void call_copy_haspodbase(HasPodBase *hasPodBase) {
+  *hasPodBase = copy(hasPodBase);
+  // WOA64-LABEL: define dso_local void @"?call_copy_haspodbase at pr47611@@YAXPEAUHasPodBase at 1@@Z"
+  // WOA64: call void @"?copy at pr47611@@YA?AUHasPodBase at 1@PEAU21@@Z"(%"struct.pr47611::HasPodBase"* inreg sret(%"struct.pr47611::HasPodBase") align 8 %{{.*}}, %"struct.pr47611::HasPodBase"* %{{.*}})
+}
+}; // namespace pr47611

diff  --git a/llvm/test/CodeGen/AArch64/arm64-windows-calls.ll b/llvm/test/CodeGen/AArch64/arm64-windows-calls.ll
index 1fee2246b751..47aefd358914 100644
--- a/llvm/test/CodeGen/AArch64/arm64-windows-calls.ll
+++ b/llvm/test/CodeGen/AArch64/arm64-windows-calls.ll
@@ -98,3 +98,80 @@ entry:
   %this1 = load %class.C*, %class.C** %this.addr, align 8
   ret void
 }
+
+; The following tests correspond to tests in
+; clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
+
+; Pod is a trivial HFA
+%struct.Pod = type { [2 x double] }
+; Not an aggregate according to C++14 spec => not HFA according to MSVC
+%struct.NotCXX14Aggregate  = type { %struct.Pod }
+; NotPod is a C++14 aggregate. But not HFA, because it contains
+; NotCXX14Aggregate (which itself is not HFA because it's not a C++14
+; aggregate).
+%struct.NotPod = type { %struct.NotCXX14Aggregate }
+
+; CHECK-LABEL: copy_pod:
+define dso_local %struct.Pod @copy_pod(%struct.Pod* %x) {
+  %x1 = load %struct.Pod, %struct.Pod* %x, align 8
+  ret %struct.Pod %x1
+  ; CHECK: ldp d0, d1, [x0]
+}
+
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* noalias nocapture writeonly, i8* noalias nocapture readonly, i64, i1 immarg)
+
+; CHECK-LABEL: copy_notcxx14aggregate:
+define dso_local void
+ at copy_notcxx14aggregate(%struct.NotCXX14Aggregate* inreg noalias sret(%struct.NotCXX14Aggregate) align 8 %agg.result,
+                        %struct.NotCXX14Aggregate* %x) {
+  %1 = bitcast %struct.NotCXX14Aggregate* %agg.result to i8*
+  %2 = bitcast %struct.NotCXX14Aggregate* %x to i8*
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %1, i8* align 8 %2, i64 16, i1 false)
+  ret void
+  ; CHECK: str q0, [x0]
+}
+
+; CHECK-LABEL: copy_notpod:
+define dso_local [2 x i64] @copy_notpod(%struct.NotPod* %x) {
+  %x1 = bitcast %struct.NotPod* %x to [2 x i64]*
+  %x2 = load [2 x i64], [2 x i64]* %x1
+  ret [2 x i64] %x2
+  ; CHECK: ldp x8, x1, [x0]
+  ; CHECK: mov x0, x8
+}
+
+ at Pod = external global %struct.Pod
+
+; CHECK-LABEL: call_copy_pod:
+define void @call_copy_pod() {
+  %x = call %struct.Pod @copy_pod(%struct.Pod* @Pod)
+  store %struct.Pod %x, %struct.Pod* @Pod
+  ret void
+  ; CHECK: bl copy_pod
+  ; CHECK-NEXT: stp d0, d1, [{{.*}}]
+}
+
+ at NotCXX14Aggregate = external global %struct.NotCXX14Aggregate
+
+; CHECK-LABEL: call_copy_notcxx14aggregate:
+define void @call_copy_notcxx14aggregate() {
+  %x = alloca %struct.NotCXX14Aggregate
+  call void @copy_notcxx14aggregate(%struct.NotCXX14Aggregate* %x, %struct.NotCXX14Aggregate* @NotCXX14Aggregate)
+  %x1 = load %struct.NotCXX14Aggregate, %struct.NotCXX14Aggregate* %x
+  store %struct.NotCXX14Aggregate %x1, %struct.NotCXX14Aggregate* @NotCXX14Aggregate
+  ret void
+  ; CHECK: bl copy_notcxx14aggregate
+  ; CHECK-NEXT: ldp {{.*}}, {{.*}}, [sp]
+}
+
+ at NotPod = external global %struct.NotPod
+
+; CHECK-LABEL: call_copy_notpod:
+define void @call_copy_notpod() {
+  %x = call [2 x i64] @copy_notpod(%struct.NotPod* @NotPod)
+  %notpod = bitcast %struct.NotPod* @NotPod to [2 x i64]*
+  store [2 x i64] %x, [2 x i64]* %notpod
+  ret void
+  ; CHECK: bl copy_notpod
+  ; CHECK-NEXT: stp x0, x1, [{{.*}}]
+}


        


More information about the cfe-commits mailing list