================
@@ -692,32 +692,44 @@ CIRGenTypes::computeRecordLayout(const RecordDecl *rd,
cir::RecordType *ty) {
assert(ty->isIncomplete() && "recomputing record layout?");
lowering.lower(/*nonVirtualBaseType=*/false);
- // If we're in C++, compute the base subobject type. For C++ records the base
- // subobject type is always set (matching classic CodeGen). For unions and
- // final classes the base subobject and complete object types are identical
- // (no tail padding can be reused), so baseTy points at the same record as
- // ty. We must still populate baseTy in those cases because callers such as
- // getStorageType(const CXXRecordDecl *) used to lay out potentially-
- // overlapping ([[no_unique_address]]) fields read it unconditionally; a
- // null baseTy would otherwise propagate as a null mlir::Type into the
- // members vector and trip the !empty() assertion in fillOutputFields.
+ // If we're in C++, compute the base subobject type. For C++ records baseTy
+ // defaults to the complete object type and is replaced by a distinct,
+ // smaller record only when the record has tail padding an enclosing
+ // [[no_unique_address]] field can reuse. We must populate baseTy even when
+ // it equals ty because callers such as getStorageType(const CXXRecordDecl *)
+ // read it unconditionally when laying out potentially-overlapping
+ // ([[no_unique_address]]) fields; a null baseTy would otherwise propagate as
+ // a null mlir::Type into the members vector and trip the !empty() assertion
+ // in fillOutputFields.
cir::RecordType baseTy;
if (llvm::isa<CXXRecordDecl>(rd)) {
baseTy = *ty;
- if (!rd->isUnion() && !rd->hasAttr<FinalAttr>() &&
- lowering.astRecordLayout.getNonVirtualSize() !=
- lowering.astRecordLayout.getSize()) {
+ // A record needs a distinct base-subobject type when its tail padding can
+ // be reused by an enclosing [[no_unique_address]] field. For a
+ // struct/class that happens when the non-virtual size differs from the
+ // complete size; for a union it happens when the data size differs from
+ // the complete size (a union has reusable tail padding when one of its
+ // members is a [[no_unique_address]] field with tail padding of its own).
+ CharUnits baseSize = rd->isUnion()
+ ? lowering.astRecordLayout.getDataSize()
+ : lowering.astRecordLayout.getNonVirtualSize();
+ // A union whose data size is zero consists entirely of
+ // [[no_unique_address]] empty members. There is no actual data to expose
+ // via a base subobject, so leave baseTy pointing at the complete type.
+ if (!baseSize.isZero() && baseSize != lowering.astRecordLayout.getSize()) {
----------------
adams381 wrote:
You're right — switched the gate to `getNonVirtualSize() != getSize()` with no
`isUnion()` special case. I'd had it backwards: for a union,
`getNonVirtualSize()` already returns the data size when there's reusable tail
padding, but stays at the one-byte minimum when the union is empty. So the
classic gate is correct on both ends — it fires for a padded union (`nvsize=1 <
size=2`) and does not fire for an all-`[[no_unique_address]]`-empty union
(`nvsize == size == 1`, data size 0). The old `getDataSize()`-based gate fired
spuriously on the empty case (`0 != 1`) and built a `getByteArrayType(0)`,
which is what the extra `isZero()` guard was compensating for; both that guard
and the matching `lowerUnion` clause are gone now.
One spot still differs from classic on purpose: the packedness assertion keeps
a union exemption. Classic derives union packedness from the data size for both
layouts, so they always agree; CIR derives it from the layout size (full size
for the complete object, data size for the base), so the two can legitimately
disagree. I left a comment to that effect.
The previous `UnionZeroDataSize` test case actually had a non-empty
`[[no_unique_address]] int` member (data size 4), so it never exercised the
empty-union path — added a genuinely all-empty union case.
https://github.com/llvm/llvm-project/pull/201428
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
