[clang] [CIR] Compute union base subobject for tail-padding reuse (PR #201428)

[via cfe-commits]

================
@@ -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