dblaikie created this revision.
dblaikie added reviewers: hansw, rnk.
Herald added subscribers: mstorsjo, kristof.beyls.
Herald added a project: All.
dblaikie requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Fixes:
Protected members, HFA: https://godbolt.org/z/zqdK7vdKc
Private members, HFA: https://godbolt.org/z/zqdK7vdKc
Non-empty base, HFA: https://godbolt.org/z/PKTz59Wev
User-provided ctor, HFA: https://godbolt.org/z/sfrTddcW6
Existing correct cases:
Empty base class, NonHFA: https://godbolt.org/z/4veY9MWP3
- correct by accident of not allowing bases at all (see non-empty base case/fix
above for counterexample)
Polymorphic: NonHFA: https://godbolt.org/z/4veY9MWP3
Trivial copy assignment, HFA: https://godbolt.org/z/Tdecj836P
Non-trivial copy assignment, NonHFA: https://godbolt.org/z/7c4bE9Whq
Non-trivial default ctor, NonHFA: https://godbolt.org/z/Tsq1EE7b7
- correct by accident of disallowing all user-provided ctors (see user-provided
non-default ctor example above for counterexample)
Trivial dtor, HFA: https://godbolt.org/z/nae999aqz
Non-trivial dtor, NonHFA: https://godbolt.org/z/69oMcshb1
Empty field, NonHFA: https://godbolt.org/z/8PTxsKKMK
- true due to checking for the absence of padding (see comment in code)
After a bunch of testing, this fixes a bunch of cases that were
incorrect. Some of the tests verify the nuances of the existing
behavior/code checks that were already present.
This was mostly motivated by cleanup from/in D133817
<https://reviews.llvm.org/D133817> which itself was
motivated by D119051 <https://reviews.llvm.org/D119051>.
By removing the incorrect use of isTrivialForAArch64MSVC here & adding
more nuance to the homogeneous testing we can more safely/confidently
make changes to the isTrivialFor(AArch64)MSVC to more properly align
with its usage anyway.
Repository:
rG LLVM Github Monorepo
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,42 @@
}
bool MicrosoftCXXABI::isPermittedToBeHomogeneousAggregate(
- const CXXRecordDecl *CXXRD) const {
+ const CXXRecordDecl *RD) 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);
+ if (!CGM.getTarget().getTriple().isAArch64())
+ return true;
+ 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.
+ // Instnead 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 (eg: 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
