dblaikie updated this revision to Diff 463379.
dblaikie added a comment.
Update comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134688/new/
https://reviews.llvm.org/D134688
Files:
clang/lib/CodeGen/MicrosoftCXXABI.cpp
clang/test/CodeGenCXX/homogeneous-aggregates.cpp
Index: clang/test/CodeGenCXX/homogeneous-aggregates.cpp
===================================================================
--- clang/test/CodeGenCXX/homogeneous-aggregates.cpp
+++ clang/test/CodeGenCXX/homogeneous-aggregates.cpp
@@ -92,7 +92,7 @@
// ARM32: define{{.*}} arm_aapcs_vfpcc void @_Z15with_empty_base16HVAWithEmptyBase(%struct.HVAWithEmptyBase %a.coerce)
void CC with_empty_base(HVAWithEmptyBase a) {}
-// FIXME: MSVC doesn't consider this an HVA because of the empty base.
+// WOA64: define dso_local void @"?with_empty_base@@YAXUHVAWithEmptyBase@@@Z"([2 x i64] %{{.*}})
// X64: define dso_local x86_vectorcallcc void @"\01_Z15with_empty_base16HVAWithEmptyBase@@16"(%struct.HVAWithEmptyBase inreg %a.coerce)
struct HVAWithEmptyBitField : Float1, Float2 {
@@ -172,4 +172,117 @@
// WOA64-LABEL: define dso_local void @"?call_copy_haspodbase@pr47611@@YAXPEAUHasPodBase@1@@Z"
// WOA64: call void @"?copy@pr47611@@YA?AUHasPodBase@1@PEAU21@@Z"(%"struct.pr47611::HasPodBase"* inreg sret(%"struct.pr47611::HasPodBase") align 8 %{{.*}}, %"struct.pr47611::HasPodBase"* noundef %{{.*}})
}
-}; // namespace pr47611
+} // namespace pr47611
+
+namespace protected_member {
+struct HFA {
+ double x;
+ double y;
+protected:
+ double z;
+};
+double foo(HFA v) { return v.x + v.y; }
+// WOA64: define dso_local noundef double @"?foo@protected_member@@YANUHFA@1@@Z"([3 x double] %{{.*}})
+}
+namespace private_member {
+struct HFA {
+ double x;
+ double y;
+private:
+ double z;
+};
+double foo(HFA v) { return v.x + v.y; }
+// WOA64: define dso_local noundef double @"?foo@private_member@@YANUHFA@1@@Z"([3 x double] %{{.*}})
+}
+namespace polymorphic {
+struct NonHFA {
+ double x;
+ double y;
+ double z;
+ virtual void f1();
+};
+double foo(NonHFA v) { return v.x + v.y; }
+// WOA64: define dso_local noundef double @"?foo@polymorphic@@YANUNonHFA@1@@Z"(%"struct.polymorphic::NonHFA"* noundef %{{.*}})
+}
+namespace trivial_copy_assignment {
+struct HFA {
+ double x;
+ double y;
+ double z;
+ HFA &operator=(const HFA&) = default;
+};
+double foo(HFA v) { return v.x + v.y; }
+// WOA64: define dso_local noundef double @"?foo@trivial_copy_assignment@@YANUHFA@1@@Z"([3 x double] %{{.*}})
+}
+namespace non_trivial_copy_assignment {
+struct NonHFA {
+ double x;
+ double y;
+ double z;
+ NonHFA &operator=(const NonHFA&);
+};
+double foo(NonHFA v) { return v.x + v.y; }
+// WOA64: define dso_local noundef double @"?foo@non_trivial_copy_assignment@@YANUNonHFA@1@@Z"(%"struct.non_trivial_copy_assignment::NonHFA"* noundef %{{.*}})
+}
+namespace user_provided_ctor {
+struct HFA {
+ double x;
+ double y;
+ double z;
+ HFA(int);
+};
+double foo(HFA v) { return v.x + v.y; }
+// WOA64: define dso_local noundef double @"?foo@user_provided_ctor@@YANUHFA@1@@Z"([3 x double] %{{.*}})
+}
+namespace trivial_dtor {
+struct HFA {
+ double x;
+ double y;
+ double z;
+ ~HFA() = default;
+};
+double foo(HFA v) { return v.x + v.y; }
+// WOA64: define dso_local noundef double @"?foo@trivial_dtor@@YANUHFA@1@@Z"([3 x double] %{{.*}})
+}
+namespace non_trivial_dtor {
+struct NonHFA {
+ double x;
+ double y;
+ double z;
+ ~NonHFA();
+};
+double foo(NonHFA v) { return v.x + v.y; }
+// WOA64: define dso_local noundef double @"?foo@non_trivial_dtor@@YANUNonHFA@1@@Z"(%"struct.non_trivial_dtor::NonHFA"* noundef %{{.*}})
+}
+namespace non_empty_base {
+struct non_empty_base { double d; };
+struct HFA : non_empty_base {
+ double x;
+ double y;
+ double z;
+};
+double foo(HFA v) { return v.x + v.y; }
+// WOA64: define dso_local noundef double @"?foo@non_empty_base@@YANUHFA@1@@Z"([4 x double] %{{.*}})
+}
+namespace empty_field {
+struct empty { };
+struct NonHFA {
+ double x;
+ double y;
+ double z;
+ empty e;
+};
+double foo(NonHFA v) { return v.x + v.y; }
+// WOA64: define dso_local noundef double @"?foo@empty_field@@YANUNonHFA@1@@Z"(%"struct.empty_field::NonHFA"* noundef %{{.*}})
+}
+namespace non_empty_field {
+struct non_empty { double d; };
+struct HFA {
+ double x;
+ double y;
+ double z;
+ non_empty e;
+};
+double foo(HFA v) { return v.x + v.y; }
+// WOA64: define dso_local noundef double @"?foo@non_empty_field@@YANUHFA@1@@Z"([4 x double] %{{.*}})
+}
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===================================================================
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -4474,10 +4474,45 @@
}
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);
+ const CXXRecordDecl *RD) const {
+ // All aggregates are permitted to be HFA on non-ARM platforms, which mostly
+ // affects vectorcall on x64/x86.
+ if (!CGM.getTarget().getTriple().isAArch64())
+ return true;
+ // MSVC Windows on Arm64 has its own rules for determining if a type is HFA
+ // that are inconsistent with the AAPCS64 ABI. The following are our best
+ // determination of those rules so far, based on observation of MSVC's
+ // behavior.
+ if (RD->isEmpty())
+ return false;
+ if (RD->isPolymorphic())
+ return false;
+ if (RD->hasNonTrivialCopyAssignment())
+ return false;
+ if (RD->hasNonTrivialDestructor())
+ return false;
+ if (RD->hasNonTrivialDefaultConstructor())
+ return false;
+ // These two are somewhat redundant given the caller
+ // (ABIInfo::isHomogeneousAggregate) checks the bases and fields, but that
+ // caller doesn't consider empty bases/fields to be non-homogenous, but it
+ // looks like Microsoft's AArch64 ABI does care about these empty types &
+ // anything containing/derived from one is non-homogeneous.
+ // Instead we could add another CXXABI entry point to query this property and
+ // have ABIInfo::isHomogeneousAggregate use that property.
+ // I don't think any other of the features listed above could be true of a
+ // base/field while not true of the outer struct. For example, if you have a
+ // base/field that has an non-trivial copy assignment/dtor/default ctor, then
+ // the outer struct's corresponding operation must be non-trivial.
+ for (const CXXBaseSpecifier &B : RD->bases()) {
+ if (const CXXRecordDecl *FRD = B.getType()->getAsCXXRecordDecl()) {
+ if (!isPermittedToBeHomogeneousAggregate(FRD))
+ return false;
+ }
+ }
+ // empty fields seem to be caught by the ABIInfo::isHomogeneousAggregate
+ // checking for padding - but maybe there are ways to end up with an empty
+ // field without padding? Not that I know of, so don't check fields here &
+ // rely on the padding check.
+ return true;
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
