Author: David Blaikie Date: 2022-10-04T20:17:29Z New Revision: 2e1c1d6d72879cafc339ad035b1b5a6d1c8cc130
URL: https://github.com/llvm/llvm-project/commit/2e1c1d6d72879cafc339ad035b1b5a6d1c8cc130 DIFF: https://github.com/llvm/llvm-project/commit/2e1c1d6d72879cafc339ad035b1b5a6d1c8cc130.diff LOG: MSVC AArch64 ABI: Homogeneous aggregates 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 which itself was motivated by 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. Differential Revision: https://reviews.llvm.org/D134688 Added: Modified: clang/lib/CodeGen/MicrosoftCXXABI.cpp clang/test/CodeGenCXX/homogeneous-aggregates.cpp Removed: ################################################################################ diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp index 539f0a6eb8cb8..76aeb7bb7b76f 100644 --- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp +++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp @@ -4476,10 +4476,45 @@ MicrosoftCXXABI::LoadVTablePtr(CodeGenFunction &CGF, Address This, } 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; } diff --git a/clang/test/CodeGenCXX/homogeneous-aggregates.cpp b/clang/test/CodeGenCXX/homogeneous-aggregates.cpp index 1e873cd7b1ace..b4cca7c9ff00b 100644 --- a/clang/test/CodeGenCXX/homogeneous-aggregates.cpp +++ b/clang/test/CodeGenCXX/homogeneous-aggregates.cpp @@ -92,7 +92,7 @@ struct HVAWithEmptyBase : Float1, Empty, Float2 { float z; }; // 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 @@ void call_copy_haspodbase(HasPodBase *hasPodBase) { // 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] %{{.*}}) +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits