uweigand created this revision.
uweigand added reviewers: craig.topper, erichkeane, jasonliu, kbarton, rnk,
asl, sunfish, t.p.northover, arsenm, asb.
Herald added subscribers: cfe-commits, wdng.
Herald added a project: clang.
The SystemZ ABI has special cases to handle structs containing just a single
floating-point member. In determining this property, there are corner cases
around empty fields. So for example, if a struct contains an "empty member" in
addition to a member of floating-point type, the struct is still considered to
contain just a single floating-point member.
In (prior versions of) C++, however, members of class type would never count as
"empty" given the C++ rules that even an empty class should be considered as
having a size of at least 1. But now with C++20, data members can be marked
with the [[no_unique_address]] attribute, in which case that rule no longer
applies. The Itanium ABI document was updated to address this new situation
(https://itanium-cxx-abi.github.io/cxx-abi/abi.html#definitions):
>
>
> empty class
> A class with no non-static data members other than empty data members,
> no unnamed bit-fields other than zero-width bit-fields, no virtual functions,
> no virtual base classes, and no non-empty non-virtual proper base classes.
>
>
> empty data member
>
> A potentially-overlapping non-static data member of empty class type.
GCC 10 has been updated across platforms to respect this new case.
This patch implements the new case in the ABI code for SystemZ. Note that I'm
changing common subroutines (isEmptyField / isEmptyRecord) that are used for
other ABIs as well. To prevent this patch from having any unintended effect on
other platforms, I've guarded the new behavior with an extra flag that is
currently only set on SystemZ.
Now I would expect that most other platforms who use isEmptyField /
isEmptyRecord / isSingleElementStruct / isHomogeneousAggregate will also have
to respect that new behavior in order to stay compatible with the respective
system ABIs (and GCC), but I'd prefer to leave this up to the maintainers of
those platforms ...
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D81583
Files:
clang/lib/CodeGen/TargetInfo.cpp
clang/test/CodeGen/systemz-abi.cpp
Index: clang/test/CodeGen/systemz-abi.cpp
===================================================================
--- clang/test/CodeGen/systemz-abi.cpp
+++ clang/test/CodeGen/systemz-abi.cpp
@@ -9,3 +9,28 @@
struct agg_float_cpp pass_agg_float_cpp(struct agg_float_cpp arg) { return arg; }
// CHECK-LABEL: define void @_Z18pass_agg_float_cpp13agg_float_cpp(%struct.agg_float_cpp* noalias sret align 4 %{{.*}}, float %{{.*}})
// SOFT-FLOAT: define void @_Z18pass_agg_float_cpp13agg_float_cpp(%struct.agg_float_cpp* noalias sret align 4 %{{.*}}, i32 %{{.*}})
+
+// A field member of empty class type in C++ makes the record nonhomogeneous,
+// unless it is marked as [[no_unique_address]];
+struct empty { };
+struct agg_nofloat_empty { float a; empty dummy; };
+struct agg_nofloat_empty pass_agg_nofloat_empty(struct agg_nofloat_empty arg) { return arg; }
+// CHECK-LABEL: define void @_Z22pass_agg_nofloat_empty17agg_nofloat_empty(%struct.agg_nofloat_empty* noalias sret align 4 %{{.*}}, i64 %{{.*}})
+// SOFT-FLOAT: define void @_Z22pass_agg_nofloat_empty17agg_nofloat_empty(%struct.agg_nofloat_empty* noalias sret align 4 %{{.*}}, i64 %{{.*}})
+struct agg_float_empty { float a; [[no_unique_address]] empty dummy; };
+struct agg_float_empty pass_agg_float_empty(struct agg_float_empty arg) { return arg; }
+// CHECK-LABEL: define void @_Z20pass_agg_float_empty15agg_float_empty(%struct.agg_float_empty* noalias sret align 4 %{{.*}}, float %{{.*}})
+// SOFT-FLOAT: define void @_Z20pass_agg_float_empty15agg_float_empty(%struct.agg_float_empty* noalias sret align 4 %{{.*}}, i32 %{{.*}})
+
+// And likewise for members of base classes.
+struct noemptybase { empty dummy; };
+struct agg_nofloat_emptybase : noemptybase { float a; };
+struct agg_nofloat_emptybase pass_agg_nofloat_emptybase(struct agg_nofloat_emptybase arg) { return arg; }
+// CHECK-LABEL: define void @_Z26pass_agg_nofloat_emptybase21agg_nofloat_emptybase(%struct.agg_nofloat_emptybase* noalias sret align 4 %{{.*}}, i64 %{{.*}})
+// SOFT-FLOAT: define void @_Z26pass_agg_nofloat_emptybase21agg_nofloat_emptybase(%struct.agg_nofloat_emptybase* noalias sret align 4 %{{.*}}, i64 %{{.*}})
+struct emptybase { [[no_unique_address]] empty dummy; };
+struct agg_float_emptybase : emptybase { float a; };
+struct agg_float_emptybase pass_agg_float_emptybase(struct agg_float_emptybase arg) { return arg; }
+// CHECK-LABEL: define void @_Z24pass_agg_float_emptybase19agg_float_emptybase(%struct.agg_float_emptybase* noalias sret align 4 %{{.*}}, float %{{.*}})
+// SOFT-FLOAT: define void @_Z24pass_agg_float_emptybase19agg_float_emptybase(%struct.agg_float_emptybase* noalias sret align 4 %{{.*}}, i32 %{{.*}})
+
Index: clang/lib/CodeGen/TargetInfo.cpp
===================================================================
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -485,12 +485,13 @@
return Ctx.getOrInsertSyncScopeID(""); /* default sync scope */
}
-static bool isEmptyRecord(ASTContext &Context, QualType T, bool AllowArrays);
+static bool isEmptyRecord(ASTContext &Context, QualType T, bool AllowArrays,
+ bool AllowNoUniqueAddr = false);
/// isEmptyField - Return true iff a the field is "empty", that is it
/// is an unnamed bit-field or an (array of) empty record(s).
static bool isEmptyField(ASTContext &Context, const FieldDecl *FD,
- bool AllowArrays) {
+ bool AllowArrays, bool AllowNoUniqueAddr = false) {
if (FD->isUnnamedBitfield())
return true;
@@ -513,16 +514,24 @@
//
// FIXME: We should use a predicate for whether this behavior is true in the
// current ABI.
- if (isa<CXXRecordDecl>(RT->getDecl()))
- return false;
+ //
+ // The exception to the above rule are fields marked with the
+ // [[no_unique_address]] attribute (since C++20). Those do count
+ // as empty according to the Itanium ABI. This property is currently
+ // only respected if the AllowNoUniqueAddr parameter is true.
+ if (isa<CXXRecordDecl>(RT->getDecl())) {
+ if (!(AllowNoUniqueAddr && FD->hasAttr<NoUniqueAddressAttr>()))
+ return false;
+ }
- return isEmptyRecord(Context, FT, AllowArrays);
+ return isEmptyRecord(Context, FT, AllowArrays, AllowNoUniqueAddr);
}
/// isEmptyRecord - Return true iff a structure contains only empty
/// fields. Note that a structure with a flexible array member is not
/// considered empty.
-static bool isEmptyRecord(ASTContext &Context, QualType T, bool AllowArrays) {
+static bool isEmptyRecord(ASTContext &Context, QualType T, bool AllowArrays,
+ bool AllowNoUniqueAddr) {
const RecordType *RT = T->getAs<RecordType>();
if (!RT)
return false;
@@ -533,11 +542,11 @@
// If this is a C++ record, check the bases first.
if (const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD))
for (const auto &I : CXXRD->bases())
- if (!isEmptyRecord(Context, I.getType(), true))
+ if (!isEmptyRecord(Context, I.getType(), true, AllowNoUniqueAddr))
return false;
for (const auto *I : RD->fields())
- if (!isEmptyField(Context, I, AllowArrays))
+ if (!isEmptyField(Context, I, AllowArrays, AllowNoUniqueAddr))
return false;
return true;
}
@@ -7153,7 +7162,7 @@
QualType Base = I.getType();
// Empty bases don't affect things either way.
- if (isEmptyRecord(getContext(), Base, true))
+ if (isEmptyRecord(getContext(), Base, true, true))
continue;
if (!Found.isNull())
@@ -7163,12 +7172,17 @@
// Check the fields.
for (const auto *FD : RD->fields()) {
- // For compatibility with GCC, ignore empty bitfields in C++ mode.
+ // For compatibility with GCC, ignore empty bitfields in C++ mode,
+ // and empty C++ classes marked with the no_unique_address attribute.
// Unlike isSingleElementStruct(), empty structure and array fields
// do count. So do anonymous bitfields that aren't zero-sized.
- if (getContext().getLangOpts().CPlusPlus &&
- FD->isZeroLengthBitField(getContext()))
- continue;
+ if (getContext().getLangOpts().CPlusPlus) {
+ if (FD->isZeroLengthBitField(getContext()))
+ continue;
+ if (FD->hasAttr<NoUniqueAddressAttr>() &&
+ isEmptyRecord(getContext(), FD->getType(), true, true))
+ continue;
+ }
// Unlike isSingleElementStruct(), arrays do not count.
// Nested structures still do though.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
