DavidTruby created this revision.
Herald added a subscriber: kristof.beyls.
DavidTruby requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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.wal...@arm.com>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92751

Files:
  clang/lib/CodeGen/CGCXXABI.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp

Index: clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
===================================================================
--- clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
+++ clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
@@ -468,3 +468,31 @@
 // WIN64-LABEL: define dso_local void @"?g@C@pr30293@@QEAAXXZ"(%"struct.pr30293::C"* {{[^,]*}} %this)
 // WIN64: declare dso_local void @"?h@C@pr30293@@UEAAXUSmallWithDtor@@@Z"(i8*, i32)
 }
+
+namespace pr47611 {
+// MSVC on Arm includes "isCXX14Aggregate" as part of its definition of
+// Homogeneous Floating-point Aggregate (HFA). Additionally, it has a different
+// 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;
+};
+// WOA64-LABEL: define dso_local %"struct.pr47611::Pod" @"?copy@pr47611@@YA?AUPod@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@pr47611@@YA?AUNotCXX14Aggregate@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@pr47611@@YA?AUNotPod@1@PEAU21@@Z"(%"struct.pr47611::NotPod"* %x)
+NotPod copy(NotPod *x) { return *x; }
+};
Index: clang/lib/CodeGen/TargetInfo.cpp
===================================================================
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -5133,6 +5133,14 @@
 
         Members += FldMembers;
       }
+
+      const auto TT = CGT.getTarget().getTriple();
+      // 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.
+      if (TT.isAArch64() && TT.isWindowsMSVCEnvironment() &&
+          !CGCXXABI::isCXX14Aggregate(CXXRD))
+        return false;
     }
 
     for (const auto *FD : RD->fields()) {
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===================================================================
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1070,30 +1070,6 @@
   return isDeletingDtor(GD);
 }
 
-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.
-  //   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 false;
-  if (RD->getNumBases() > 0)
-    return false;
-  if (RD->isPolymorphic())
-    return false;
-  if (RD->hasNonTrivialCopyAssignment())
-    return false;
-  for (const CXXConstructorDecl *Ctor : RD->ctors())
-    if (Ctor->isUserProvided())
-      return false;
-  if (RD->hasNonTrivialDestructor())
-    return false;
-  return true;
-}
-
 bool MicrosoftCXXABI::classifyReturnType(CGFunctionInfo &FI) const {
   const CXXRecordDecl *RD = FI.getReturnType()->getAsCXXRecordDecl();
   if (!RD)
Index: clang/lib/CodeGen/CGCXXABI.h
===================================================================
--- clang/lib/CodeGen/CGCXXABI.h
+++ clang/lib/CodeGen/CGCXXABI.h
@@ -625,6 +625,8 @@
   virtual std::pair<llvm::Value *, const CXXRecordDecl *>
   LoadVTablePtr(CodeGenFunction &CGF, Address This,
                 const CXXRecordDecl *RD) = 0;
+
+  static bool isCXX14Aggregate(const CXXRecordDecl *RD);
 };
 
 // Create an instance of a C++ ABI class:
Index: clang/lib/CodeGen/CGCXXABI.cpp
===================================================================
--- clang/lib/CodeGen/CGCXXABI.cpp
+++ clang/lib/CodeGen/CGCXXABI.cpp
@@ -310,3 +310,27 @@
   return AddedStructorArgCounts(AddedArgs.Prefix.size(),
                                 AddedArgs.Suffix.size());
 }
+
+bool CGCXXABI::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.
+  //   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 false;
+  if (RD->getNumBases() > 0)
+    return false;
+  if (RD->isPolymorphic())
+    return false;
+  if (RD->hasNonTrivialCopyAssignment())
+    return false;
+  for (const CXXConstructorDecl *Ctor : RD->ctors())
+    if (Ctor->isUserProvided())
+      return false;
+  if (RD->hasNonTrivialDestructor())
+    return false;
+  return true;
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D92751: Precondition i... David Truby via Phabricator via cfe-commits

Reply via email to