[CIR] Fix record layout for a union with no storage type by adams381 · Pull Request #213591 · llvm/llvm-project

adams381 · 2026-08-03T04:53:32Z

A union whose CIR type ends up with no members keeps its whole size in its
padding field, and UnionType::getTypeSizeInBits returned early in exactly that
case, before reaching the padding. A union need not look empty in the source to
land there: a lone zero-length bitfield is dropped during lowering, leaving the
same no-storage state.

A record embedding such a union was then laid out wrong. In an unpacked record
insertPadding pads whenever the end of the members placed so far, rounded up
to the next member's alignment, falls short of that member's offset, so a union
measuring zero earns a pad the AST layout does not have. In C++,
struct { union {} e; int x; } loaded x from byte 8 rather than 4, and an
array of that struct had a 12-byte stride, not 8. With the union alignas(16),
the load came from byte 32 rather than 16.

The zero also reached lowerUnion, which sizes a union's padding as its layout
size less its storage member's, so union { union {} e; } emitted a two-byte
type for a one-byte union.

Sum the storage and padding contributions instead of returning early. The
has-storage path is unchanged, and a C empty union stays at size zero because it
has no padding field to add.

UnionType::getABIAlignment keeps its early return. Union padding is always a
char or an array of char, so folding it in cannot change the alignment of
anything CIRGen emits.

A union whose CIR type ends up with no members keeps its whole size in its padding field, and `UnionType::getTypeSizeInBits` returned early in exactly that case, before reaching the padding. A union need not look empty in the source to land there: a lone zero-length bitfield is dropped during lowering, leaving the same no-storage state. A record embedding such a union was then laid out wrong. In an unpacked record `insertPadding` pads whenever the end of the members placed so far, rounded up to the next member's alignment, falls short of that member's offset, so a union measuring zero earns a pad the AST layout does not have. In C++, `struct { union {} e; int x; }` loaded `x` from byte 8 rather than 4, and an array of that struct had a 12-byte stride, not 8. With the union `alignas(16)`, the load came from byte 32 rather than 16. The zero also reached `lowerUnion`, which sizes a union's padding as its layout size less its storage member's, so `union { union {} e; }` emitted a two-byte type for a one-byte union. Sum the storage and padding contributions instead of returning early. The has-storage path is unchanged, and a C empty union stays at size zero because it has no padding field to add.

llvmorg-github-actions · 2026-08-03T04:54:06Z

@llvm/pr-subscribers-clangir

Author: Adam Smith (adams381)

Changes

A union whose CIR type ends up with no members keeps its whole size in its
padding field, and UnionType::getTypeSizeInBits returned early in exactly that
case, before reaching the padding. A union need not look empty in the source to
land there: a lone zero-length bitfield is dropped during lowering, leaving the
same no-storage state.

A record embedding such a union was then laid out wrong. In an unpacked record
insertPadding pads whenever the end of the members placed so far, rounded up
to the next member's alignment, falls short of that member's offset, so a union
measuring zero earns a pad the AST layout does not have. In C++,
struct { union {} e; int x; } loaded x from byte 8 rather than 4, and an
array of that struct had a 12-byte stride, not 8. With the union alignas(16),
the load came from byte 32 rather than 16.

The zero also reached lowerUnion, which sizes a union's padding as its layout
size less its storage member's, so union { union {} e; } emitted a two-byte
type for a one-byte union.

Sum the storage and padding contributions instead of returning early. The
has-storage path is unchanged, and a C empty union stays at size zero because it
has no padding field to add.

UnionType::getABIAlignment keeps its early return. Union padding is always a
char or an array of char, so folding it in cannot change the alignment of
anything CIRGen emits.

Full diff: https://github.com/llvm/llvm-project/pull/213591.diff

2 Files Affected:

(modified) clang/lib/CIR/Dialect/IR/CIRTypes.cpp (+7-6)
(added) clang/test/CIR/CodeGen/empty-union-record-layout.cpp (+134)

diff --git a/clang/lib/CIR/Dialect/IR/CIRTypes.cpp b/clang/lib/CIR/Dialect/IR/CIRTypes.cpp
index c1f3d3dc6cca5..fba7bf6ac0fda 100644
--- a/clang/lib/CIR/Dialect/IR/CIRTypes.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRTypes.cpp
@@ -727,15 +727,16 @@ StructType::getABIAlignment(const ::mlir::DataLayout &dataLayout,
 llvm::TypeSize
 UnionType::getTypeSizeInBits(const mlir::DataLayout &dataLayout,
                              mlir::DataLayoutEntryListRef params) const {
-  mlir::Type storage = getUnionStorageType(dataLayout);
-  if (!storage)
-    return llvm::TypeSize::getFixed(0);
+  // A union whose member list came out empty has no storage type, so whatever
+  // size it has lives entirely in the padding field below.  Sum both.
+  llvm::TypeSize size = llvm::TypeSize::getFixed(0);
+  if (mlir::Type storage = getUnionStorageType(dataLayout))
+    size += dataLayout.getTypeSizeInBits(storage);
   // The padding field holds enough bytes to bring the total up to the AST
   // layout size (set by lowerUnion from the ASTRecordLayout).  Include it so
   // getTypeSize agrees with the {storage, padding} LLVM struct that
-  // LowerToLLVM emits; without it a containing record adds spurious tail
-  // padding via insertPadding, making sizeof and array GEPs wrong.
-  llvm::TypeSize size = dataLayout.getTypeSizeInBits(storage);
+  // LowerToLLVM emits.  Without it a containing record adds spurious padding
+  // via insertPadding, making the emitted record's size and its GEPs wrong.
   if (mlir::Type pad = getPadding())
     size += dataLayout.getTypeSizeInBits(pad);
   return size;
diff --git a/clang/test/CIR/CodeGen/empty-union-record-layout.cpp b/clang/test/CIR/CodeGen/empty-union-record-layout.cpp
new file mode 100644
index 0000000000000..4fca4a063e387
--- /dev/null
+++ b/clang/test/CIR/CodeGen/empty-union-record-layout.cpp
@@ -0,0 +1,134 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o - | FileCheck %s --check-prefix=CIR
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o - | FileCheck %s --check-prefixes=LLVM,LLVMCIR
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o - | FileCheck %s --check-prefixes=LLVM,OGCG
+
+union Memberless {};
+
+union alignas(16) MemberlessOver {};
+
+// A zero-length bitfield is dropped during lowering, so this union reaches the
+// same no-storage state despite declaring a member.
+union OnlyZeroBitfield {
+  int : 0;
+};
+
+struct Leading {
+  Memberless e;
+  int x;
+};
+
+struct Trailing {
+  int x;
+  Memberless e;
+};
+
+// A union whose only member is itself storage-less.  This one HAS a storage
+// type, so it is the storage member's reported size that must be right, and a
+// wrapping record cannot expose the error because the trailing field is
+// realigned regardless.
+union OnlyMemberless {
+  Memberless e;
+};
+
+struct Middle {
+  int a;
+  Memberless e;
+  int b;
+};
+
+struct LeadingOver {
+  MemberlessOver e;
+  int x;
+};
+
+struct LeadingZeroBitfield {
+  OnlyZeroBitfield e;
+  int x;
+};
+
+OnlyMemberless onlyMemberless;
+Leading lead;
+Trailing trail;
+Middle mid;
+LeadingOver leadOver;
+LeadingZeroBitfield leadZero;
+Leading leadArr[2];
+
+// CIR-DAG: !rec_Memberless = !cir.union<"Memberless" {}, padding = {!u8i}>
+// CIR-DAG: !rec_MemberlessOver = !cir.union<"MemberlessOver" {}, padding = {!cir.array<!u8i x 16>}>
+// CIR-DAG: !rec_OnlyMemberless = !cir.union<"OnlyMemberless" {!rec_Memberless}>
+// CIR-DAG: !rec_Leading = !cir.struct<"Leading" {!rec_Memberless, !s32i}>
+// CIR-DAG: !rec_Trailing = !cir.struct<"Trailing" {!s32i, !rec_Memberless}>
+// CIR-DAG: !rec_Middle = !cir.struct<"Middle" {!s32i, !rec_Memberless, !s32i}>
+// CIR-DAG: !rec_LeadingOver = !cir.struct<"LeadingOver" padded {!rec_MemberlessOver, !s32i, !cir.array<!u8i x 12>}>
+// CIR-DAG: !rec_OnlyZeroBitfield = !cir.union<"OnlyZeroBitfield" {}, padding = {!u8i}>
+// CIR-DAG: !rec_LeadingZeroBitfield = !cir.struct<"LeadingZeroBitfield" {!rec_OnlyZeroBitfield, !s32i}>
+
+// Neither path carries a pad for the union's own bytes, though they spell those
+// bytes differently.
+// LLVMCIR-DAG: %struct.Leading = type { %union.Memberless, i32 }
+// LLVMCIR-DAG: %struct.Trailing = type { i32, %union.Memberless }
+// LLVMCIR-DAG: %struct.Middle = type { i32, %union.Memberless, i32 }
+// LLVMCIR-DAG: %struct.LeadingZeroBitfield = type { %union.OnlyZeroBitfield, i32 }
+// LLVMCIR-DAG: %struct.LeadingOver = type { %union.MemberlessOver, i32, [12 x i8] }
+// OGCG-DAG:    %struct.Leading = type { [4 x i8], i32 }
+// OGCG-DAG:    %struct.Trailing = type { i32, [4 x i8] }
+// OGCG-DAG:    %struct.Middle = type { i32, [4 x i8], i32 }
+// OGCG-DAG:    %struct.LeadingZeroBitfield = type { [4 x i8], i32 }
+// OGCG-DAG:    %struct.LeadingOver = type { [16 x i8], i32, [12 x i8] }
+// LLVM-DAG:    %union.OnlyMemberless = type { %union.Memberless }
+// LLVM-DAG:    @lead = global %struct.Leading zeroinitializer, align 4
+// LLVM-DAG:    @leadOver = global %struct.LeadingOver zeroinitializer, align 16
+
+// The union occupies one byte, so the int follows at offset 4.
+int getLeading() { return lead.x; }
+
+// CIR:  cir.func{{.*}} @_Z10getLeadingv()
+// CIR:    %[[L:.*]] = cir.get_global @lead : !cir.ptr<!rec_Leading>
+// CIR:    %{{.*}} = cir.get_member %[[L]][1] {name = "x"} : !cir.ptr<!rec_Leading> -> !cir.ptr<!s32i>
+// LLVM: define dso_local noundef i32 @_Z10getLeadingv()
+// LLVM:   load i32, ptr getelementptr inbounds nuw (i8, ptr @lead, i64 4), align 4
+
+// With the union last, the size it contributes lands in the record's tail.
+int getTrailing() { return trail.x; }
+
+// CIR:  cir.func{{.*}} @_Z11getTrailingv()
+// CIR:    %[[T:.*]] = cir.get_global @trail : !cir.ptr<!rec_Trailing>
+// CIR:    %{{.*}} = cir.get_member %[[T]][0] {name = "x"} : !cir.ptr<!rec_Trailing> -> !cir.ptr<!s32i>
+// LLVM: define dso_local noundef i32 @_Z11getTrailingv()
+// LLVM:   load i32, ptr @trail, align 4
+
+// The union sits between two fields, so only the field AFTER it moves.
+int getMiddle() { return mid.b; }
+
+// CIR:  cir.func{{.*}} @_Z9getMiddlev()
+// CIR:    %[[M:.*]] = cir.get_global @mid : !cir.ptr<!rec_Middle>
+// CIR:    %{{.*}} = cir.get_member %[[M]][2] {name = "b"} : !cir.ptr<!rec_Middle> -> !cir.ptr<!s32i>
+// LLVM: define dso_local noundef i32 @_Z9getMiddlev()
+// LLVM:   load i32, ptr getelementptr inbounds nuw (i8, ptr @mid, i64 8), align 4
+
+// An over-aligned union spells its size as an array of char rather than a
+// single char, and the record embedding it has real tail padding of its own.
+int getLeadingOver() { return leadOver.x; }
+
+// CIR:  cir.func{{.*}} @_Z14getLeadingOverv()
+// CIR:    %[[O:.*]] = cir.get_global @leadOver : !cir.ptr<!rec_LeadingOver>
+// CIR:    %{{.*}} = cir.get_member %[[O]][1] {name = "x"} : !cir.ptr<!rec_LeadingOver> -> !cir.ptr<!s32i>
+// LLVM: define dso_local noundef i32 @_Z14getLeadingOverv()
+// LLVM:   load i32, ptr getelementptr inbounds nuw (i8, ptr @leadOver, i64 16), align 16
+
+// The dropped bitfield leaves no storage member, so this behaves like Leading.
+int getLeadingZeroBitfield() { return leadZero.x; }
+
+// CIR:  cir.func{{.*}} @_Z22getLeadingZeroBitfieldv()
+// CIR:    %[[Z:.*]] = cir.get_global @leadZero : !cir.ptr<!rec_LeadingZeroBitfield>
+// CIR:    %{{.*}} = cir.get_member %[[Z]][1] {name = "x"} : !cir.ptr<!rec_LeadingZeroBitfield> -> !cir.ptr<!s32i>
+// LLVM: define dso_local noundef i32 @_Z22getLeadingZeroBitfieldv()
+// LLVM:   load i32, ptr getelementptr inbounds nuw (i8, ptr @leadZero, i64 4), align 4
+
+// The element stride is 8, so the second element's int is at offset 12.
+int getArray() { return leadArr[1].x; }
+
+// CIR:  cir.func{{.*}} @_Z8getArrayv()
+// LLVM: define dso_local noundef i32 @_Z8getArrayv()
+// LLVM:   load i32, ptr getelementptr inbounds nuw (i8, ptr @leadArr, i64 12), align 4

llvmorg-github-actions · 2026-08-03T04:54:07Z

@llvm/pr-subscribers-clang

Author: Adam Smith (adams381)

Changes

A union whose CIR type ends up with no members keeps its whole size in its
padding field, and UnionType::getTypeSizeInBits returned early in exactly that
case, before reaching the padding. A union need not look empty in the source to
land there: a lone zero-length bitfield is dropped during lowering, leaving the
same no-storage state.

A record embedding such a union was then laid out wrong. In an unpacked record
insertPadding pads whenever the end of the members placed so far, rounded up
to the next member's alignment, falls short of that member's offset, so a union
measuring zero earns a pad the AST layout does not have. In C++,
struct { union {} e; int x; } loaded x from byte 8 rather than 4, and an
array of that struct had a 12-byte stride, not 8. With the union alignas(16),
the load came from byte 32 rather than 16.

The zero also reached lowerUnion, which sizes a union's padding as its layout
size less its storage member's, so union { union {} e; } emitted a two-byte
type for a one-byte union.

Sum the storage and padding contributions instead of returning early. The
has-storage path is unchanged, and a C empty union stays at size zero because it
has no padding field to add.

UnionType::getABIAlignment keeps its early return. Union padding is always a
char or an array of char, so folding it in cannot change the alignment of
anything CIRGen emits.

Full diff: https://github.com/llvm/llvm-project/pull/213591.diff

2 Files Affected:

(modified) clang/lib/CIR/Dialect/IR/CIRTypes.cpp (+7-6)
(added) clang/test/CIR/CodeGen/empty-union-record-layout.cpp (+134)

diff --git a/clang/lib/CIR/Dialect/IR/CIRTypes.cpp b/clang/lib/CIR/Dialect/IR/CIRTypes.cpp
index c1f3d3dc6cca5..fba7bf6ac0fda 100644
--- a/clang/lib/CIR/Dialect/IR/CIRTypes.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRTypes.cpp
@@ -727,15 +727,16 @@ StructType::getABIAlignment(const ::mlir::DataLayout &dataLayout,
 llvm::TypeSize
 UnionType::getTypeSizeInBits(const mlir::DataLayout &dataLayout,
                              mlir::DataLayoutEntryListRef params) const {
-  mlir::Type storage = getUnionStorageType(dataLayout);
-  if (!storage)
-    return llvm::TypeSize::getFixed(0);
+  // A union whose member list came out empty has no storage type, so whatever
+  // size it has lives entirely in the padding field below.  Sum both.
+  llvm::TypeSize size = llvm::TypeSize::getFixed(0);
+  if (mlir::Type storage = getUnionStorageType(dataLayout))
+    size += dataLayout.getTypeSizeInBits(storage);
   // The padding field holds enough bytes to bring the total up to the AST
   // layout size (set by lowerUnion from the ASTRecordLayout).  Include it so
   // getTypeSize agrees with the {storage, padding} LLVM struct that
-  // LowerToLLVM emits; without it a containing record adds spurious tail
-  // padding via insertPadding, making sizeof and array GEPs wrong.
-  llvm::TypeSize size = dataLayout.getTypeSizeInBits(storage);
+  // LowerToLLVM emits.  Without it a containing record adds spurious padding
+  // via insertPadding, making the emitted record's size and its GEPs wrong.
   if (mlir::Type pad = getPadding())
     size += dataLayout.getTypeSizeInBits(pad);
   return size;
diff --git a/clang/test/CIR/CodeGen/empty-union-record-layout.cpp b/clang/test/CIR/CodeGen/empty-union-record-layout.cpp
new file mode 100644
index 0000000000000..4fca4a063e387
--- /dev/null
+++ b/clang/test/CIR/CodeGen/empty-union-record-layout.cpp
@@ -0,0 +1,134 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o - | FileCheck %s --check-prefix=CIR
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o - | FileCheck %s --check-prefixes=LLVM,LLVMCIR
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o - | FileCheck %s --check-prefixes=LLVM,OGCG
+
+union Memberless {};
+
+union alignas(16) MemberlessOver {};
+
+// A zero-length bitfield is dropped during lowering, so this union reaches the
+// same no-storage state despite declaring a member.
+union OnlyZeroBitfield {
+  int : 0;
+};
+
+struct Leading {
+  Memberless e;
+  int x;
+};
+
+struct Trailing {
+  int x;
+  Memberless e;
+};
+
+// A union whose only member is itself storage-less.  This one HAS a storage
+// type, so it is the storage member's reported size that must be right, and a
+// wrapping record cannot expose the error because the trailing field is
+// realigned regardless.
+union OnlyMemberless {
+  Memberless e;
+};
+
+struct Middle {
+  int a;
+  Memberless e;
+  int b;
+};
+
+struct LeadingOver {
+  MemberlessOver e;
+  int x;
+};
+
+struct LeadingZeroBitfield {
+  OnlyZeroBitfield e;
+  int x;
+};
+
+OnlyMemberless onlyMemberless;
+Leading lead;
+Trailing trail;
+Middle mid;
+LeadingOver leadOver;
+LeadingZeroBitfield leadZero;
+Leading leadArr[2];
+
+// CIR-DAG: !rec_Memberless = !cir.union<"Memberless" {}, padding = {!u8i}>
+// CIR-DAG: !rec_MemberlessOver = !cir.union<"MemberlessOver" {}, padding = {!cir.array<!u8i x 16>}>
+// CIR-DAG: !rec_OnlyMemberless = !cir.union<"OnlyMemberless" {!rec_Memberless}>
+// CIR-DAG: !rec_Leading = !cir.struct<"Leading" {!rec_Memberless, !s32i}>
+// CIR-DAG: !rec_Trailing = !cir.struct<"Trailing" {!s32i, !rec_Memberless}>
+// CIR-DAG: !rec_Middle = !cir.struct<"Middle" {!s32i, !rec_Memberless, !s32i}>
+// CIR-DAG: !rec_LeadingOver = !cir.struct<"LeadingOver" padded {!rec_MemberlessOver, !s32i, !cir.array<!u8i x 12>}>
+// CIR-DAG: !rec_OnlyZeroBitfield = !cir.union<"OnlyZeroBitfield" {}, padding = {!u8i}>
+// CIR-DAG: !rec_LeadingZeroBitfield = !cir.struct<"LeadingZeroBitfield" {!rec_OnlyZeroBitfield, !s32i}>
+
+// Neither path carries a pad for the union's own bytes, though they spell those
+// bytes differently.
+// LLVMCIR-DAG: %struct.Leading = type { %union.Memberless, i32 }
+// LLVMCIR-DAG: %struct.Trailing = type { i32, %union.Memberless }
+// LLVMCIR-DAG: %struct.Middle = type { i32, %union.Memberless, i32 }
+// LLVMCIR-DAG: %struct.LeadingZeroBitfield = type { %union.OnlyZeroBitfield, i32 }
+// LLVMCIR-DAG: %struct.LeadingOver = type { %union.MemberlessOver, i32, [12 x i8] }
+// OGCG-DAG:    %struct.Leading = type { [4 x i8], i32 }
+// OGCG-DAG:    %struct.Trailing = type { i32, [4 x i8] }
+// OGCG-DAG:    %struct.Middle = type { i32, [4 x i8], i32 }
+// OGCG-DAG:    %struct.LeadingZeroBitfield = type { [4 x i8], i32 }
+// OGCG-DAG:    %struct.LeadingOver = type { [16 x i8], i32, [12 x i8] }
+// LLVM-DAG:    %union.OnlyMemberless = type { %union.Memberless }
+// LLVM-DAG:    @lead = global %struct.Leading zeroinitializer, align 4
+// LLVM-DAG:    @leadOver = global %struct.LeadingOver zeroinitializer, align 16
+
+// The union occupies one byte, so the int follows at offset 4.
+int getLeading() { return lead.x; }
+
+// CIR:  cir.func{{.*}} @_Z10getLeadingv()
+// CIR:    %[[L:.*]] = cir.get_global @lead : !cir.ptr<!rec_Leading>
+// CIR:    %{{.*}} = cir.get_member %[[L]][1] {name = "x"} : !cir.ptr<!rec_Leading> -> !cir.ptr<!s32i>
+// LLVM: define dso_local noundef i32 @_Z10getLeadingv()
+// LLVM:   load i32, ptr getelementptr inbounds nuw (i8, ptr @lead, i64 4), align 4
+
+// With the union last, the size it contributes lands in the record's tail.
+int getTrailing() { return trail.x; }
+
+// CIR:  cir.func{{.*}} @_Z11getTrailingv()
+// CIR:    %[[T:.*]] = cir.get_global @trail : !cir.ptr<!rec_Trailing>
+// CIR:    %{{.*}} = cir.get_member %[[T]][0] {name = "x"} : !cir.ptr<!rec_Trailing> -> !cir.ptr<!s32i>
+// LLVM: define dso_local noundef i32 @_Z11getTrailingv()
+// LLVM:   load i32, ptr @trail, align 4
+
+// The union sits between two fields, so only the field AFTER it moves.
+int getMiddle() { return mid.b; }
+
+// CIR:  cir.func{{.*}} @_Z9getMiddlev()
+// CIR:    %[[M:.*]] = cir.get_global @mid : !cir.ptr<!rec_Middle>
+// CIR:    %{{.*}} = cir.get_member %[[M]][2] {name = "b"} : !cir.ptr<!rec_Middle> -> !cir.ptr<!s32i>
+// LLVM: define dso_local noundef i32 @_Z9getMiddlev()
+// LLVM:   load i32, ptr getelementptr inbounds nuw (i8, ptr @mid, i64 8), align 4
+
+// An over-aligned union spells its size as an array of char rather than a
+// single char, and the record embedding it has real tail padding of its own.
+int getLeadingOver() { return leadOver.x; }
+
+// CIR:  cir.func{{.*}} @_Z14getLeadingOverv()
+// CIR:    %[[O:.*]] = cir.get_global @leadOver : !cir.ptr<!rec_LeadingOver>
+// CIR:    %{{.*}} = cir.get_member %[[O]][1] {name = "x"} : !cir.ptr<!rec_LeadingOver> -> !cir.ptr<!s32i>
+// LLVM: define dso_local noundef i32 @_Z14getLeadingOverv()
+// LLVM:   load i32, ptr getelementptr inbounds nuw (i8, ptr @leadOver, i64 16), align 16
+
+// The dropped bitfield leaves no storage member, so this behaves like Leading.
+int getLeadingZeroBitfield() { return leadZero.x; }
+
+// CIR:  cir.func{{.*}} @_Z22getLeadingZeroBitfieldv()
+// CIR:    %[[Z:.*]] = cir.get_global @leadZero : !cir.ptr<!rec_LeadingZeroBitfield>
+// CIR:    %{{.*}} = cir.get_member %[[Z]][1] {name = "x"} : !cir.ptr<!rec_LeadingZeroBitfield> -> !cir.ptr<!s32i>
+// LLVM: define dso_local noundef i32 @_Z22getLeadingZeroBitfieldv()
+// LLVM:   load i32, ptr getelementptr inbounds nuw (i8, ptr @leadZero, i64 4), align 4
+
+// The element stride is 8, so the second element's int is at offset 12.
+int getArray() { return leadArr[1].x; }
+
+// CIR:  cir.func{{.*}} @_Z8getArrayv()
+// LLVM: define dso_local noundef i32 @_Z8getArrayv()
+// LLVM:   load i32, ptr getelementptr inbounds nuw (i8, ptr @leadArr, i64 12), align 4

erichkeane · 2026-08-03T13:18:18Z

@@ -0,0 +1,134 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o - | FileCheck %s --check-prefix=CIR


We have tons of record-layout test files, why does this have to be a new one?

This was an oversight. This PR is prework (in addition to #213357) split out of the larger, still pending, union support PR. I guess when I was splitting the work out I didn't check properly for where to place the tests. I'll move this to empty-union.cpp where it belongs.

erichkeane · 2026-08-03T13:20:28Z

+Leading leadArr[2];
+
+// CIR-DAG: !rec_Memberless = !cir.union<"Memberless" {}, padding = {!u8i}>
+// CIR-DAG: !rec_MemberlessOver = !cir.union<"MemberlessOver" {}, padding = {!cir.array<!u8i x 16>}>


What is the layout of this type in the various LLVM types? Why is it not added there?

The layout of this type is { [16 x i8] }. In the version of the file you reviewed there was no use of the union, so classic didn't emit it. Not checking was an oversight. In the move to the pre-existing empty-union.cpp mentioned above, the same type already exists and has a use. There are now checks for each case.

These went into a new file that re-declared two unions empty-union.cpp already had. Moving them there meant making the type-alias checks -DAG, since the aliases stop coming out in declaration order once seven more types are added. I also collapsed the LLVM and OGCG prefixes where the two agree. OnlyZeroBitfield had no check on its own lowered type. Classic never names it, since LeadingZeroBitfield covers the union with a char array, so useZeroBitfield() gives it a use and one LLVM check now covers both.

erichkeane

A few test updates + streamline the comment, else lgtm.

erichkeane · 2026-08-03T20:47:24Z

+// CIR: cir.func {{.*}}@_Z15useZeroBitfieldv()
+// CIR:   cir.alloca "e" align(1) : !cir.ptr<!rec_OnlyZeroBitfield>
+// LLVM: define {{.*}} void @_Z15useZeroBitfieldv()
+// LLVMCIR:   alloca %union.OnlyZeroBitfield, i64 1, align 1


This pattern (with alignment) is irrelevantly different. We could probably be better in CIR lowering to skip the '1' count, but just check both with the alloca + type name.

You are right. I've made that change and it is much cleaner now.

erichkeane · 2026-08-03T20:47:30Z

-// LLVM:   alloca %union.EmptyAligned, i64 1, align 16
-// OGCG: define {{.*}} void @_Z15useEmptyAlignedv()
-// OGCG:   alloca %union.EmptyAligned, align 16
+// LLVMCIR:   alloca %union.EmptyAligned, i64 1, align 16


Same as above.

erichkeane · 2026-08-03T20:50:44Z

+  llvm::TypeSize size = llvm::TypeSize::getFixed(0);
+  if (mlir::Type storage = getUnionStorageType(dataLayout))
+    size += dataLayout.getTypeSizeInBits(storage);
  // The padding field holds enough bytes to bring the total up to the AST


Rather than 2 long comments, can we just do a bit of a 'top post' here describing the functionality here?

This isn't complex enough to require 7 lines of comment. In reality, this whole function isn't much more than 1-liner...

Actually. tis a touch of a shame that getTypeSizeInBits doesn't just return 0 on null mlir::Type, else this becomes:

return dataLayout.getTypeSizeInBits(getUnionStorageType(dataLayout)) + dataLayout.getTypeSizeInBits(getPadding());

Something to keep an eye out if we have a similar need in the future, might be worth extracting the `if !type, return 0, else get type-size-in-bits' into its own function if this keeps happening.

I've simplified the comments to a single top comment that is now a three-liner. The effect is subtle enough I don't want it to get lost again.

I agree about getTypeSizeInBits. I'll try to keep that in mind.

Simplify the comments on `UnionType::getTypeSizeInBits`. This wording keeps the important information without running on. Use wildcards to combine the test checks where irrelevantly different into a single `LLVM` check. Assisted-by: Cursor / claude-opus-5

erichkeane · 2026-08-03T21:50:49Z

+// CIR keeps the union's own named type as the record's field and leaves the
+// bytes after it to the LLVM struct layout.  Classic covers the union together
+// with those bytes in one char array.
+// LLVMCIR-DAG: %struct.Leading = type { %union.Empty, i32 }


Not really necessary here (though do if you want), but 'interleaving' the LLVMCIR and OGCG lines can often be really nice, such that the same-struct is next to eachother in each.

It breaks up the examples a bit, but makes it a bit easier to compare 1 vs the next. That said, with this group, I was able to manage.

adams381 requested review from andykaylor, bcardosolopes and xlauko as code owners August 3, 2026 04:53

llvmorg-github-actions Bot added clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project labels Aug 3, 2026

adams381 requested review from erichkeane and lanza August 3, 2026 04:56

erichkeane reviewed Aug 3, 2026

View reviewed changes

erichkeane approved these changes Aug 3, 2026

View reviewed changes

adams381 merged commit 7a0afd3 into llvm:main Aug 3, 2026
12 checks passed

		@@ -0,0 +1,134 @@
		// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o - \| FileCheck %s --check-prefix=CIR

Uh oh!

Conversation

adams381 commented Aug 3, 2026

Uh oh!

llvmorg-github-actions Bot commented Aug 3, 2026

Uh oh!

llvmorg-github-actions Bot commented Aug 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants