[clang] 2e1c1d6 - MSVC AArch64 ABI: Homogeneous aggregates
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 4 13:17:41 PDT 2022
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 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"* 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 at protected_member@@YANUHFA at 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 at private_member@@YANUHFA at 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 at polymorphic@@YANUNonHFA at 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 at trivial_copy_assignment@@YANUHFA at 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 at non_trivial_copy_assignment@@YANUNonHFA at 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 at user_provided_ctor@@YANUHFA at 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 at trivial_dtor@@YANUHFA at 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 at non_trivial_dtor@@YANUNonHFA at 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 at non_empty_base@@YANUHFA at 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 at empty_field@@YANUNonHFA at 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 at non_empty_field@@YANUHFA at 1@@Z"([4 x double] %{{.*}})
+}
More information about the cfe-commits
mailing list