================
@@ -692,21 +692,28 @@ 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();
+ if (baseSize != lowering.astRecordLayout.getSize()) {
----------------
adams381 wrote:
Classic CodeGen's gate at `CGRecordLayoutBuilder.cpp:1091` is simply `if
(Builder.Layout.getNonVirtualSize() != Builder.Layout.getSize())` — no
`isUnion()` guard and no `FinalAttr` guard. The old CIR code added both, and
both were wrong. The `FinalAttr` guard was unsound because a `final` class can
still contain `[[no_unique_address]]` members whose base subobject type differs
from their complete type (the `final` keyword prevents *derivation*, not
tail-padding reuse by an enclosing field). Removing it aligns CIR with classic.
The `isUnion()` guard was removed because we now handle unions — using
`getDataSize()` as the base size for unions, exactly as classic does at line
315.
https://github.com/llvm/llvm-project/pull/201428
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
