https://github.com/adams381 updated https://github.com/llvm/llvm-project/pull/218718
>From 2da5cc7367af883ea1952d8f0067db45e7b321e2 Mon Sep 17 00:00:00 2001 From: Adam Smith <[email protected]> Date: Tue, 25 Aug 2026 08:57:34 -0700 Subject: [PATCH 1/4] [CIR] Accept a union with an ABI-empty member isSupportedType rejected any union containing an ABI-empty member outright. Dropping the reject alone is not enough. A union mixing a data-free member that spans the record with a bit-field access unit can still mis-lower. The new accept rule requires that a data-supplying member span the record whenever a bit-field access unit is present. mapCIRType's union loop now only maps members that hold data for the ABI, so an unnamed bit-field's storage is not mapped as a field either. Assisted-by: Cursor / claude-opus-5 --- .../Transforms/CallConvLoweringPass.cpp | 29 ++- .../call-conv-lowering-x86_64-empty.cpp | 92 +++++++ .../abi-lowering/x86_64-aggregate-nyi.cir | 32 ++- .../Transforms/abi-lowering/x86_64-union.cir | 226 ++++++++++++++++++ 4 files changed, 351 insertions(+), 28 deletions(-) diff --git a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp index c9bf699ad5561..f96c87f44ec94 100644 --- a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp +++ b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp @@ -79,7 +79,8 @@ namespace { // vectors, a padded record reached through a named bit-field access unit, a // record holding an empty-for-ABI member that occupies bytes or a zero-sized // one off its own alignment, a union no member of which spans its declared -// size, and a union with an empty-record member are reported NYI by +// size, and a union whose only spanning member is a bit-field access unit +// are reported NYI by // classifyX86_64Function so an unsupported signature fails the pass instead of // being misclassified. //===----------------------------------------------------------------------===// @@ -227,12 +228,15 @@ static bool isSupportedType(mlir::Type ty, const DataLayout &dl) { }; if (!llvm::any_of(members, spansRecord)) return false; - // Classic sizes a union's coercion from the bytes that hold data, so an - // empty member contributes none. The library instead reduces the union - // to one member, picked by alignment and then by size, and coerces from - // that member: an empty one can win either comparison and widen the - // coercion past what classic emits. - if (llvm::any_of(members, memberIsEmptyRecord)) + // A bit-field access unit's width can understate the bit-fields it + // holds. When the union has such a unit, a spanning member is not + // enough on its own: it must also supply data, either the unit + // itself or another member. + llvm::ArrayRef<cir::RecordMemberKind> kinds = recTy.getMemberKinds(); + if (llvm::any_of(kinds, cir::isBitFieldAccessUnit) && + !llvm::any_of(members, [&](mlir::Type m) { + return spansRecord(m) && !memberIsEmptyRecord(m); + })) return false; } } else if (recTy.getPadded() && reachesNamedBitFieldUnit(recTy)) { @@ -384,9 +388,14 @@ static const llvm::abi::Type *mapCIRType(mlir::Type type, // the whole union rather than just the member the classifier reduces // it to. if (recTy.isUnion()) { - for (mlir::Type fieldTy : recTy.getMembers()) - fields.push_back(llvm::abi::FieldInfo( - mapCIRType(fieldTy, typeMapper, dl, modOp))); + // Only data members are classified. An unnamed bit-field's storage + // is a member here but contributes no class in classic CodeGen, so + // mapping it would pass an argument classic drops. + for (auto [fieldTy, kind] : + llvm::zip_equal(recTy.getMembers(), recTy.getMemberKinds())) + if (cir::holdsDataForABI(fieldTy, kind)) + fields.push_back(llvm::abi::FieldInfo( + mapCIRType(fieldTy, typeMapper, dl, modOp))); return tb.getUnionType(fields, sizeBits, align, llvm::abi::StructPacking::Default, flags); } diff --git a/clang/test/CIR/CodeGen/call-conv-lowering-x86_64-empty.cpp b/clang/test/CIR/CodeGen/call-conv-lowering-x86_64-empty.cpp index 126470331ce01..30999935ef9d0 100644 --- a/clang/test/CIR/CodeGen/call-conv-lowering-x86_64-empty.cpp +++ b/clang/test/CIR/CodeGen/call-conv-lowering-x86_64-empty.cpp @@ -27,6 +27,16 @@ struct FloatEmptyFirst { Empty e; float a; }; struct alignas(32) Big32 {}; union UBits { unsigned : 3; }; union UNone {}; +union UEmptyInt { Empty e; int i; }; +union UEmptyAligned { Aligned e; int i; }; +union UArrEmpty { Empty a[2]; char c; }; +union UEmptyOnly { Empty e; }; +union UEmptyDouble { Empty e; double d; }; +union UEmptyBytes { Empty e; char c[8]; }; +union UBigEmpty { Big32 e; int i; }; +union UEmptyBaseMem { HasEmptyBase e; int i; }; +union UValue { Empty mono; int i; long long ll; double d; const char *s; }; +struct ArgStore { UValue value; unsigned char type; }; // An empty class is passed in no register at all. int takeEmpty(Empty v, int k) { return k; } @@ -161,6 +171,88 @@ int takeUNone(UNone v, int k) { return k; } // CIR: cir.func {{.*}}@_Z9takeUNone5UNonei(%arg0: !s32i {{.*}}) -> (!s32i // LLVM: define dso_local noundef i32 @_Z9takeUNone5UNonei(i32 noundef %{{[^,]+}}) +// A union with an empty member coerces from the member that supplies bytes, +// not from the union's size. +int takeUEmptyInt(UEmptyInt v) { return v.i; } + +// CIR: cir.func {{.*}}@_Z13takeUEmptyInt9UEmptyInt(%arg0: !s32i {{.*}}) -> (!s32i +// LLVM: define dso_local noundef i32 @_Z13takeUEmptyInt9UEmptyInt(i32 %{{[^,]+}}) + +// Here the empty member's alignment (16) outranks the int's (4), so the same +// rule matters more: a member supplying no bytes still cannot decide the +// storage type, and the 16-byte union coerces to the int's eightbyte rather +// than widening to i64. +int takeUEmptyAligned(UEmptyAligned v) { return v.i; } + +// CIR: cir.func {{.*}}@_Z17takeUEmptyAligned13UEmptyAligned(%arg0: !s32i {{.*}}) -> (!s32i +// LLVM: define dso_local noundef i32 @_Z17takeUEmptyAligned13UEmptyAligned(i32 %{{[^,]+}}) + +// An alignment tie is broken by size, so an array of empty records outranks +// the byte of data while holding none itself. +int takeUArrEmpty(UArrEmpty v) { return v.c; } + +// CIR: cir.func {{.*}}@_Z13takeUArrEmpty9UArrEmpty(%arg0: !s8i {{.*}}) -> (!s32i +// LLVM: define dso_local noundef i32 @_Z13takeUArrEmpty9UArrEmpty(i8 %{{[^,]+}}) + +// A union of nothing but an empty member is dropped, like an empty class. +int takeUEmptyOnly(UEmptyOnly v, int k) { return k; } + +// CIR: cir.func {{.*}}@_Z14takeUEmptyOnly10UEmptyOnlyi(%arg0: !s32i {{.*}}) -> (!s32i +// LLVM: define dso_local noundef i32 @_Z14takeUEmptyOnly10UEmptyOnlyi(i32 noundef %{{[^,]+}}) + +// Skipping the empty member leaves the class to the member that remains, so +// this passes in an SSE register rather than an integer one. +double takeUEmptyDouble(UEmptyDouble v) { return v.d; } + +// CIR: cir.func {{.*}}@_Z16takeUEmptyDouble12UEmptyDouble(%arg0: !cir.double {{.*}}) -> (!cir.double +// LLVM: define dso_local noundef double @_Z16takeUEmptyDouble12UEmptyDouble(double %{{[^,]+}}) + +// Where the data member fills the eightbyte there is nothing to narrow. +int takeUEmptyBytes(UEmptyBytes v) { return v.c[0]; } + +// CIR: cir.func {{.*}}@_Z15takeUEmptyBytes11UEmptyBytes(%arg0: !u64i {{.*}}) -> (!s32i +// LLVM: define dso_local noundef i32 @_Z15takeUEmptyBytes11UEmptyBytes(i64 %{{[^,]+}}) + +// Past two eightbytes SysV says memory whatever the content, so the empty +// member changes nothing here. +int takeUBigEmpty(UBigEmpty v, int k) { return k; } + +// CIR: cir.func {{.*}}@_Z13takeUBigEmpty9UBigEmptyi(%arg0: !cir.ptr<!rec_UBigEmpty> {llvm.align = 32 : i64, llvm.byval = !rec_UBigEmpty, llvm.noalias, llvm.noundef}{{.*}}, %arg1: !s32i {{.*}}) -> (!s32i +// LLVM-CIR: define dso_local noundef i32 @_Z13takeUBigEmpty9UBigEmptyi(ptr noalias noundef byval(%union.UBigEmpty) align 32 %{{[^,]+}}, i32 noundef %{{[^,]+}}) +// LLVM-OGCG: define dso_local noundef i32 @_Z13takeUBigEmpty9UBigEmptyi(ptr noundef byval(%union.UBigEmpty) align 32 %{{[^,]+}}, i32 noundef %{{[^,]+}}) + +// The same union returned uses sret at that alignment. +UBigEmpty retUBigEmpty() { return UBigEmpty{}; } + +// CIR: cir.func {{.*}}@_Z12retUBigEmptyv(%arg0: !cir.ptr<!rec_UBigEmpty> {llvm.align = 32 : i64, llvm.dead_on_unwind, llvm.noalias, llvm.sret = !rec_UBigEmpty, llvm.writable} +// LLVM: define dso_local void @_Z12retUBigEmptyv(ptr dead_on_unwind noalias writable sret(%union.UBigEmpty) align 32 %{{[^,]+}}) + +// Emptiness reaches the union member through a base class as well. +int takeUEmptyBaseMem(UEmptyBaseMem v) { return v.i; } + +// CIR: cir.func {{.*}}@_Z17takeUEmptyBaseMem13UEmptyBaseMem(%arg0: !s32i {{.*}}) -> (!s32i +// LLVM: define dso_local noundef i32 @_Z17takeUEmptyBaseMem13UEmptyBaseMem(i32 %{{[^,]+}}) + +// Several scalars alongside one empty member: the widest of the scalars decides +// the coercion. +long long takeUValue(UValue v) { return v.ll; } + +// CIR: cir.func {{.*}}@_Z10takeUValue6UValue(%arg0: !s64i {{.*}}) -> (!s64i +// LLVM: define dso_local noundef i64 @_Z10takeUValue6UValue(i64 %{{[^,]+}}) + +// The same union as a struct member, where the struct's eightbytes are what +// gets classified. +long long takeArgStore(ArgStore a) { return a.value.ll; } + +// CIR: cir.func {{.*}}@_Z12takeArgStore8ArgStore(%arg0: !s64i {{.*}}, %arg1: !u8i {{.*}}) -> (!s64i +// LLVM: define dso_local noundef i64 @_Z12takeArgStore8ArgStore(i64 %{{[^,]+}}, i8 %{{[^,]+}}) + +// The union returned by value round-trips through its coercion. +UValue retUValue() { return UValue{}; } + +// CIR: cir.func {{.*}}@_Z9retUValuev() -> !s64i +// LLVM: define dso_local i64 @_Z9retUValuev() + // An empty return is dropped to void. Empty retEmpty() { return Empty{}; } diff --git a/clang/test/CIR/Transforms/abi-lowering/x86_64-aggregate-nyi.cir b/clang/test/CIR/Transforms/abi-lowering/x86_64-aggregate-nyi.cir index 74abbdb0de626..f2a159c560385 100644 --- a/clang/test/CIR/Transforms/abi-lowering/x86_64-aggregate-nyi.cir +++ b/clang/test/CIR/Transforms/abi-lowering/x86_64-aggregate-nyi.cir @@ -17,12 +17,10 @@ !cir.struct<"NamedPlusZeroWidth" {bitfield !u8i, pad !cir.array<!u8i x 3>, bitfield !cir.array<!s32i x 0>, data !s32i, pad !cir.array<!u8i x 8>}> -!rec_E = !cir.struct<"E" {pad !u8i}> -!rec_EOver = !cir.struct<"EOver" {pad !cir.array<!u8i x 16>}> !rec_UPadByte = !cir.union<"UPadByte" {data !u8i}, padding = {!cir.array<!u8i x 3>}> -!rec_UEmptyOnly = !cir.union<"UEmptyOnly" {data !rec_E}> -!rec_UEmptyOver = !cir.union<"UEmptyOver" {data !rec_EOver, data !s32i}, padding = {!cir.array<!u8i x 12>}> -!rec_UArrEmpty = !cir.union<"UArrEmpty" {data !cir.array<!rec_E x 2>, data !s8i}> +!rec_E = !cir.struct<"E" {pad !u8i}> +!rec_UBitEmpty = !cir.union<"UBitEmpty" {data !cir.array<!rec_E x 4>, bitfield !u8i}> +!rec_UEmptyNarrow = !cir.union<"UEmptyNarrow" {data !rec_E, data !s16i}, padding = {!cir.array<!u8i x 2>}> !rec_S1 = !cir.struct<"S1" {data !s16i, data !s16i, data !s16i}> !rec_AtomicWrapper = !cir.struct<{data !rec_S1, pad !cir.array<!s8i x 2>}> !rec_BitPad = !cir.struct<"BitPad" {bitfield !u8i, pad !cir.array<!u8i x 15>}> @@ -123,27 +121,25 @@ module attributes { // CHECK: not yet implemented for type '!cir.union<"UPadByte" - // A union with an empty member coerces wider than classic, so it is left NYI - // rather than accepted along with the empty class itself. - cir.func @take_union_empty_over(%arg0: !rec_UEmptyOver) { + // A bit-field access unit's width can understate the bit-fields it holds. + // Here the only member that spans the union (the empty-record array) + // supplies no bytes, so the unit alone would coerce to i8 where classic + // gives i32. + cir.func @take_bitfield_empty_span(%arg0: !rec_UBitEmpty) { cir.return } - // CHECK: not yet implemented for type '!cir.union<"UEmptyOver" + // CHECK: not yet implemented for type '!cir.union<"UBitEmpty" - // Deferred with it, though this one holds no data at all. - cir.func @take_union_empty_only(%arg0: !rec_UEmptyOnly) { + // An empty member does not exempt a union from the spanning rule above: + // as with take_short_storage_union, neither member reaches the union's + // 4 declared bytes. + cir.func @take_empty_narrow_union(%arg0: !rec_UEmptyNarrow) { cir.return } - // CHECK: not yet implemented for type '!cir.union<"UEmptyOnly" - - // An array of empty records supplies no bytes the union coercion can read. - cir.func @take_union_arr_empty(%arg0: !rec_UArrEmpty) { - cir.return - } + // CHECK: not yet implemented for type '!cir.union<"UEmptyNarrow" - // CHECK: not yet implemented for type '!cir.union<"UArrEmpty" // A bit-field access unit is narrower than the declaration it holds, so // classifying around this record's padding would coerce to i8 where classic // CodeGen widens to i64. diff --git a/clang/test/CIR/Transforms/abi-lowering/x86_64-union.cir b/clang/test/CIR/Transforms/abi-lowering/x86_64-union.cir index da0d9e20529a9..8f457bc95f0f9 100644 --- a/clang/test/CIR/Transforms/abi-lowering/x86_64-union.cir +++ b/clang/test/CIR/Transforms/abi-lowering/x86_64-union.cir @@ -21,6 +21,28 @@ !rec_UEmpty = !cir.union<"UEmpty" {}, padding = {!u8i}> !rec_UNoRegs = !cir.union<"UNoRegs" {data !s32i, data !cir.float}> !rec_SWithUnion = !cir.struct<"SWithUnion" {data !rec_UIntFloat, data !s32i}> +!rec_E = !cir.struct<"E" {pad !u8i}> +!rec_E2 = !cir.struct<"E2" {pad !u8i}> +!rec_EOver = !cir.struct<"EOver" {pad !cir.array<!u8i x 16>}> +!rec_EBig = !cir.struct<"EBig" {pad !cir.array<!u8i x 32>}> +!rec_UEmptyInt = !cir.union<"UEmptyInt" {data !rec_E, data !s32i}> +!rec_UTwoEmpty = !cir.union<"UTwoEmpty" {data !rec_E, data !rec_E2, data !s32i}> +!rec_UEmptyTwoEightbytes = !cir.union<"UEmptyTwoEightbytes" {data !rec_E, data !cir.array<!s8i x 16>}> +!rec_SWithEmptyUnion = !cir.struct<"SWithEmptyUnion" {data !rec_UEmptyInt, data !s32i}> +!rec_UEmptyOver = !cir.union<"UEmptyOver" {data !rec_EOver, data !s32i}, padding = {!cir.array<!u8i x 12>}> +!rec_UEmptyOnly = !cir.union<"UEmptyOnly" {data !rec_E}> +!rec_UEmptyOverOnly = !cir.union<"UEmptyOverOnly" {data !rec_EOver}> +!rec_UEmptyDouble = !cir.union<"UEmptyDouble" {data !rec_E, data !cir.double}> +!rec_UEmptyBytes = !cir.union<"UEmptyBytes" {data !rec_E, data !cir.array<!s8i x 8>}> +!rec_UArrEmpty = !cir.union<"UArrEmpty" {data !cir.array<!rec_E x 2>, data !s8i}> +!rec_UArr8Empty = !cir.union<"UArr8Empty" {data !cir.array<!rec_E x 8>, data !s8i}> +!rec_UBigEmptyInt = !cir.union<"UBigEmptyInt" {data !rec_EBig, data !s32i}, padding = {!cir.array<!u8i x 28>}> +!rec_UEmptyUnnamedBits = !cir.union<"UEmptyUnnamedBits" {data !cir.array<!rec_E x 4>, empty !cir.array<!u8i x 3>}> +!rec_SFloats = !cir.struct<"SFloats" {data !cir.float, data !cir.float}> +!rec_UEmptyFloats = !cir.union<"UEmptyFloats" {data !rec_E, data !rec_SFloats}> +!u32i = !cir.int<u, 32> +!rec_UBitSpans = !cir.union<"UBitSpans" {bitfield !u32i}> +!rec_UBitPlusLong = !cir.union<"UBitPlusLong" {bitfield !u32i, data !s64i}> module attributes { cir.triple = "x86_64-unknown-linux-gnu", @@ -32,6 +54,27 @@ module attributes { arg_passing_kind = can_pass_in_regs, has_trivial_dtor = true, record_align = 32>, SOverAligned = #cir.record_layout< + arg_passing_kind = can_pass_in_regs, has_trivial_dtor = true, + record_align = 32>, + E = #cir.record_layout< + arg_passing_kind = can_pass_in_regs, has_trivial_dtor = true, + record_align = 1>, + E2 = #cir.record_layout< + arg_passing_kind = can_pass_in_regs, has_trivial_dtor = true, + record_align = 1>, + EOver = #cir.record_layout< + arg_passing_kind = can_pass_in_regs, has_trivial_dtor = true, + record_align = 16>, + EBig = #cir.record_layout< + arg_passing_kind = can_pass_in_regs, has_trivial_dtor = true, + record_align = 32>, + UEmptyOver = #cir.record_layout< + arg_passing_kind = can_pass_in_regs, has_trivial_dtor = true, + record_align = 16>, + UEmptyOverOnly = #cir.record_layout< + arg_passing_kind = can_pass_in_regs, has_trivial_dtor = true, + record_align = 16>, + UBigEmptyInt = #cir.record_layout< arg_passing_kind = can_pass_in_regs, has_trivial_dtor = true, record_align = 32>}, dlti.dl_spec = #dlti.dl_spec< @@ -180,6 +223,169 @@ module attributes { // CHECK: cir.alloca "u" align(1) : !cir.ptr<!rec_UEmpty> // CHECK: cir.return{{$}} + // An empty member does not reject a union: the int is the only member + // supplying bytes, so it is what the coercion is built from. + cir.func @take_empty_int(%arg0: !rec_UEmptyInt) { + cir.return + } + + // CHECK: cir.func{{.*}} @take_empty_int(%arg0: !s32i) + // CHECK: %[[CAST:.*]] = cir.cast bitcast %{{.*}} : !cir.ptr<!s32i> -> !cir.ptr<!rec_UEmptyInt> + + // The empty member's declared alignment (16) outranks the int's (4). A + // member supplying no bytes is skipped when the union's storage type is + // chosen, so this 16-byte union still coerces to the int's eightbyte + // instead of widening to i64. + cir.func @take_empty_over(%arg0: !rec_UEmptyOver) { + cir.return + } + + // CHECK: cir.func{{.*}} @take_empty_over(%arg0: !s32i) + // CHECK: %[[SLOT:.*]] = cir.alloca "coerce" align(4) : !cir.ptr<!rec_UEmptyOver> + // CHECK: %[[CAST:.*]] = cir.cast bitcast %[[SLOT]] : !cir.ptr<!rec_UEmptyOver> -> !cir.ptr<!s32i> + + // A union of nothing but an empty member classifies Ignore, the same as the + // member-less shape above. + cir.func @take_empty_only(%arg0: !rec_UEmptyOnly) { + cir.return + } + + // CHECK: cir.func{{.*}} @take_empty_only() + + // Still Ignore at two eightbytes, where the empty member is over-aligned. + cir.func @take_empty_over_only(%arg0: !rec_UEmptyOverOnly) { + cir.return + } + + // CHECK: cir.func{{.*}} @take_empty_over_only() + + // Skipping the empty member does not force an integer coercion. The + // member that remains decides the eightbyte's class. + cir.func @take_empty_double(%arg0: !rec_UEmptyDouble) { + cir.return + } + + // CHECK: cir.func{{.*}} @take_empty_double(%arg0: !cir.double) + + // Where the other member does fill the eightbyte there is nothing to narrow, + // so this stays i64. + cir.func @take_empty_bytes(%arg0: !rec_UEmptyBytes) { + cir.return + } + + // CHECK: cir.func{{.*}} @take_empty_bytes(%arg0: !u64i) + + // The reduction breaks an alignment tie by size, so an array of empty + // records beats the one byte of data without holding any itself. + cir.func @take_arr_empty(%arg0: !rec_UArrEmpty) { + cir.return + } + + // CHECK: cir.func{{.*}} @take_arr_empty(%arg0: !s8i) + // CHECK: %[[CAST:.*]] = cir.cast bitcast %{{.*}} : !cir.ptr<!rec_UArrEmpty> -> !cir.ptr<!s8i> + + // The same array spanning a whole eightbyte, where reading the union's size + // instead of its data would coerce to i64. + cir.func @take_arr8_empty(%arg0: !rec_UArr8Empty) { + cir.return + } + + // CHECK: cir.func{{.*}} @take_arr8_empty(%arg0: !s8i) + + // Past two eightbytes SysV says memory whatever the content, so the empty + // member changes nothing and the union is passed byval. + cir.func @take_big_empty_int(%arg0: !rec_UBigEmptyInt) { + cir.return + } + + // CHECK: cir.func{{.*}} @take_big_empty_int(%arg0: !cir.ptr<!rec_UBigEmptyInt> {llvm.align = 32 : i64, llvm.byval = !rec_UBigEmptyInt, llvm.noalias, llvm.noundef}) + + // More than one empty member is skipped the same way. + cir.func @take_two_empty(%arg0: !rec_UTwoEmpty) { + cir.return + } + + // CHECK: cir.func{{.*}} @take_two_empty(%arg0: !s32i) + + // Only data members are classified, so the unnamed bit-field's storage is + // not mapped and the union is left holding nothing that supplies bytes. + cir.func @take_empty_unnamed_bits(%arg0: !rec_UEmptyUnnamedBits) { + cir.return + } + + // CHECK: cir.func{{.*}} @take_empty_unnamed_bits() + + // The remaining member decides the class, so a pair of floats in one + // eightbyte still coerces to a vector rather than an integer. + cir.func @take_empty_floats(%arg0: !rec_UEmptyFloats) { + cir.return + } + + // CHECK: cir.func{{.*}} @take_empty_floats(%arg0: !cir.vector<2 x !cir.float>) + + // A named bit-field access unit that spans the union is itself the data + // the coercion reads from, so it is accepted on its own. + cir.func @take_bit_spans(%arg0: !rec_UBitSpans) { + cir.return + } + + // CHECK: cir.func{{.*}} @take_bit_spans(%arg0: !u32i) + + // Here the bit-field access unit does not span, but the long does, so the + // union is accepted from the long's eightbyte rather than the unit's. + cir.func @take_bit_plus_long(%arg0: !rec_UBitPlusLong) { + cir.return + } + + // CHECK: cir.func{{.*}} @take_bit_plus_long(%arg0: !s64i) + + // Both eightbytes classify INTEGER and are flattened into one argument each, + // so the empty member does not disturb a multi-eightbyte coercion. + cir.func @take_empty_two_eightbytes(%arg0: !rec_UEmptyTwoEightbytes) { + cir.return + } + + // CHECK: cir.func{{.*}} @take_empty_two_eightbytes(%arg0: !u64i, %arg1: !u64i) + + // SWithEmptyUnion embeds UEmptyInt as a member, so the struct's own + // eightbyte classification has to look through the union: this pins that + // the enclosing struct still coerces correctly, not just the bare union. + cir.func @take_struct_with_empty_union(%arg0: !rec_SWithEmptyUnion) { + cir.return + } + + // CHECK: cir.func{{.*}} @take_struct_with_empty_union(%arg0: !u64i) + + // A coerced union return round-trips through the coercion type. + cir.func @ret_empty_int(%arg0: !rec_UEmptyInt) -> !rec_UEmptyInt { + %0 = cir.alloca "u" align(4) : !cir.ptr<!rec_UEmptyInt> + cir.store %arg0, %0 : !rec_UEmptyInt, !cir.ptr<!rec_UEmptyInt> + %1 = cir.load %0 : !cir.ptr<!rec_UEmptyInt>, !rec_UEmptyInt + cir.return %1 : !rec_UEmptyInt + } + + // CHECK: cir.func{{.*}} @ret_empty_int(%arg0: !s32i) -> !s32i + // CHECK: cir.return %{{.*}} : !s32i + + // A union of only empty members is dropped from the return as well. + cir.func @ret_empty_only() -> !rec_UEmptyOnly { + %0 = cir.alloca "u" align(1) : !cir.ptr<!rec_UEmptyOnly> + %1 = cir.load %0 : !cir.ptr<!rec_UEmptyOnly>, !rec_UEmptyOnly + cir.return %1 : !rec_UEmptyOnly + } + + // CHECK: cir.func{{.*}} @ret_empty_only() + // CHECK: cir.return{{$}} + + // The call site coerces the same way the callee expects. + cir.func @call_empty_int(%arg0: !rec_UEmptyInt) { + cir.call @take_empty_int(%arg0) : (!rec_UEmptyInt) -> () + cir.return + } + + // CHECK: cir.func{{.*}} @call_empty_int(%arg0: !s32i) + // CHECK: cir.call @take_empty_int(%{{.*}}) : (!s32i) -> () + // A union the record layout marks as unable to pass in registers goes // indirect without byval, however small it is. cir.func @take_no_regs(%arg0: !rec_UNoRegs) { @@ -270,6 +476,26 @@ module attributes { // LLVM: define void @take_struct_over_aligned(ptr noalias noundef byval(%struct.SOverAligned) align 32 %{{.+}}) // LLVM: define void @take_empty() // LLVM: define void @ret_empty() +// LLVM: define void @take_empty_int(i32 %{{.+}}) +// LLVM: define void @take_empty_over(i32 %{{.+}}) +// LLVM: define void @take_empty_only() +// LLVM: define void @take_empty_over_only() +// LLVM: define void @take_empty_double(double %{{.+}}) +// LLVM: define void @take_empty_bytes(i64 %{{.+}}) +// LLVM: define void @take_arr_empty(i8 %{{.+}}) +// LLVM: define void @take_arr8_empty(i8 %{{.+}}) +// LLVM: define void @take_big_empty_int(ptr noalias noundef byval(%union.UBigEmptyInt) align 32 %{{.+}}) +// LLVM: define void @take_two_empty(i32 %{{.+}}) +// LLVM: define void @take_empty_unnamed_bits() +// LLVM: define void @take_empty_floats(<2 x float> %{{.+}}) +// LLVM: define void @take_bit_spans(i32 %{{.+}}) +// LLVM: define void @take_bit_plus_long(i64 %{{.+}}) +// LLVM: define void @take_empty_two_eightbytes(i64 %{{.+}}, i64 %{{.+}}) +// LLVM: define void @take_struct_with_empty_union(i64 %{{.+}}) +// LLVM: define i32 @ret_empty_int(i32 %{{.+}}) +// LLVM: define void @ret_empty_only() +// LLVM: define void @call_empty_int(i32 %{{.+}}) +// LLVM: call void @take_empty_int(i32 %{{.+}}) // LLVM: define void @take_no_regs(ptr byref(%union.UNoRegs) align 4 %{{.+}}) // LLVM: define void @take_struct_with_union(i64 %{{.+}}) // LLVM: define i32 @ret_int_float(i32 %{{.+}}) >From 99cc98830fad1df34351abfa6491a5aa95468783 Mon Sep 17 00:00:00 2001 From: Adam Smith <[email protected]> Date: Tue, 25 Aug 2026 17:09:08 -0500 Subject: [PATCH 2/4] Update clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp Co-authored-by: Andy Kaylor <[email protected]> --- clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp index f96c87f44ec94..aab5c1f723e75 100644 --- a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp +++ b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp @@ -389,8 +389,8 @@ static const llvm::abi::Type *mapCIRType(mlir::Type type, // it to. if (recTy.isUnion()) { // Only data members are classified. An unnamed bit-field's storage - // is a member here but contributes no class in classic CodeGen, so - // mapping it would pass an argument classic drops. + // is a member here but contributes no class, so mapping it would + // pass an argument that should be dropped. for (auto [fieldTy, kind] : llvm::zip_equal(recTy.getMembers(), recTy.getMemberKinds())) if (cir::holdsDataForABI(fieldTy, kind)) >From 1d4835749cd6534fb0d6e04cf488194c60379208 Mon Sep 17 00:00:00 2001 From: Adam Smith <[email protected]> Date: Tue, 25 Aug 2026 15:54:08 -0700 Subject: [PATCH 3/4] [CIR] Rewrap the x86_64 classifier bridge comment This paragraph lost its shape over a couple of merges and ended up with a short orphaned line in the middle. Rewrapping the whole block lets classifyX86_64Function land mid-line so the lines fill evenly. The wording is untouched, only the line breaks moved. Assisted-by: Cursor / claude-opus-5 --- .../Transforms/CallConvLoweringPass.cpp | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp index 4ff69fee623f0..4e850fe114397 100644 --- a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp +++ b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp @@ -70,19 +70,18 @@ namespace { //===----------------------------------------------------------------------===// // x86_64 System V classifier bridge // -// Maps CIR types to llvm::abi::Type, runs the LLVM ABI Lowering Library's -// SysV x86_64 classifier, and converts the result back into the -// dialect-agnostic mlir::abi::FunctionClassification that CIRABIRewriteContext -// consumes. Integer (including `_BitInt` up to 128 bits) / pointer / vtable -// pointer / bool / floating-point scalars are handled, as are struct / union -// / array aggregates, `_Complex`, and a fixed-width vector whose width is a -// power of two. Other vectors, a padded record reached through a named -// bit-field access unit, a record holding an empty-for-ABI member that -// occupies bytes or a zero-sized one off its own alignment, a union no -// member of which spans its declared size, and a union whose only spanning -// member is a bit-field access unit are reported NYI by -// classifyX86_64Function so an unsupported signature fails the pass instead -// of being misclassified. +// Maps CIR types to llvm::abi::Type, runs the LLVM ABI Lowering Library's SysV +// x86_64 classifier, and converts the result back into the dialect-agnostic +// mlir::abi::FunctionClassification that CIRABIRewriteContext consumes. +// Integer (including `_BitInt` up to 128 bits) / pointer / vtable pointer / +// bool / floating-point scalars are handled, as are struct / union / array +// aggregates, `_Complex`, and a fixed-width vector whose width is a power +// of two. Other vectors, a padded record reached through a named bit-field +// access unit, a record holding an empty-for-ABI member that occupies +// bytes or a zero-sized one off its own alignment, a union no member of +// which spans its declared size, and a union whose only spanning member is +// a bit-field access unit are reported NYI by classifyX86_64Function so an +// unsupported signature fails the pass instead of being misclassified. //===----------------------------------------------------------------------===// /// Whether a struct's declared argument-passing kind (from the module's >From 0fecb762791b1698937d9e51eaa6982d8f8b2b48 Mon Sep 17 00:00:00 2001 From: Adam Smith <[email protected]> Date: Tue, 25 Aug 2026 16:34:30 -0700 Subject: [PATCH 4/4] [CIR] Diagnose a union whose spanning member supplies no data The accept rule and mapCIRType disagreed on what counts as data. The rule asked !memberIsEmptyRecord, the mapper asked holdsDataForABI, and the two part ways on an access unit holding only unnamed bit-fields: the rule counted it, the mapper dropped it. So a union could be accepted on the strength of a member the classifier never saw, and one holding a 3-bit named field beside a 64-bit unnamed one came out i8 where classic gives i64, with no diagnostic. Ask both questions of the same member so the two agree. A union holding a 20-bit named field beside a 64-bit unnamed one goes NYI as well, and that one is correct today at i64, so this costs a little coverage until the unnamed unit can carry its bytes into the classification. Assisted-by: Cursor / claude-opus-5 --- .../Transforms/CallConvLoweringPass.cpp | 20 +++++++++------- .../abi-lowering/x86_64-aggregate-nyi.cir | 23 +++++++++++++++++++ 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp index 4e850fe114397..7520c67e103f1 100644 --- a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp +++ b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp @@ -77,11 +77,11 @@ namespace { // bool / floating-point scalars are handled, as are struct / union / array // aggregates, `_Complex`, and a fixed-width vector whose width is a power // of two. Other vectors, a padded record reached through a named bit-field -// access unit, a record holding an empty-for-ABI member that occupies -// bytes or a zero-sized one off its own alignment, a union no member of -// which spans its declared size, and a union whose only spanning member is -// a bit-field access unit are reported NYI by classifyX86_64Function so an -// unsupported signature fails the pass instead of being misclassified. +// access unit, a record holding an empty-for-ABI member that occupies bytes +// or a zero-sized one off its own alignment, a union no member of which spans +// its declared size, and a union with a bit-field access unit no spanning +// member of which supplies data are reported NYI by classifyX86_64Function +// so an unsupported signature fails the pass instead of being misclassified. //===----------------------------------------------------------------------===// /// Whether a struct's declared argument-passing kind (from the module's @@ -240,9 +240,13 @@ static bool isSupportedType(mlir::Type ty, const DataLayout &dl) { // itself or another member. llvm::ArrayRef<cir::RecordMemberKind> kinds = recTy.getMemberKinds(); if (llvm::any_of(kinds, cir::isBitFieldAccessUnit) && - !llvm::any_of(members, [&](mlir::Type m) { - return spansRecord(m) && !memberIsEmptyRecord(m); - })) + !llvm::any_of(llvm::zip_equal(members, kinds), + [&](const auto &pair) { + auto [memberTy, kind] = pair; + return spansRecord(memberTy) && + cir::holdsDataForABI(memberTy, kind) && + !memberIsEmptyRecord(memberTy); + })) return false; } } else if (recTy.getPadded() && reachesNamedBitFieldUnit(recTy)) { diff --git a/clang/test/CIR/Transforms/abi-lowering/x86_64-aggregate-nyi.cir b/clang/test/CIR/Transforms/abi-lowering/x86_64-aggregate-nyi.cir index e708e2531feb7..63697d8f6d0eb 100644 --- a/clang/test/CIR/Transforms/abi-lowering/x86_64-aggregate-nyi.cir +++ b/clang/test/CIR/Transforms/abi-lowering/x86_64-aggregate-nyi.cir @@ -5,6 +5,7 @@ !s32i = !cir.int<s, 32> !u8i = !cir.int<u, 8> !u32i = !cir.int<u, 32> +!u64i = !cir.int<u, 64> !rec_UOverAligned = !cir.union<"UOverAligned" {data !s32i}, padding = {!cir.array<!u8i x 12>}> !rec_UShortStorage = !cir.union<"UShortStorage" {data !s16i, data !cir.array<!s8i x 3>}, padding = {!cir.array<!u8i x 2>}> !rec_UByteBlobs = !cir.union<"UByteBlobs" {data !u8i, data !u8i}, padding = {!cir.array<!u8i x 3>}> @@ -19,6 +20,9 @@ !rec_UPadByte = !cir.union<"UPadByte" {data !u8i}, padding = {!cir.array<!u8i x 3>}> !rec_E = !cir.struct<"E" {pad !u8i}> !rec_UBitEmpty = !cir.union<"UBitEmpty" {data !cir.array<!rec_E x 4>, bitfield !u8i}> +!rec_UBitUnnamed = !cir.union<"UBitUnnamed" {bitfield !u8i, empty !u64i}> +!rec_UWideBitUnnamed = + !cir.union<"UWideBitUnnamed" {bitfield !cir.array<!u8i x 3>, empty !u64i}> !rec_UEmptyNarrow = !cir.union<"UEmptyNarrow" {data !rec_E, data !s16i}, padding = {!cir.array<!u8i x 2>}> !rec_S1 = !cir.struct<"S1" {data !s16i, data !s16i, data !s16i}> !rec_AtomicWrapper = !cir.struct<{data !rec_S1, pad !cir.array<!s8i x 2>}> @@ -121,6 +125,25 @@ module attributes { // CHECK: not yet implemented for type '!cir.union<"UBitEmpty" + // The spanning member here is an access unit holding only unnamed + // bit-fields, which supplies no data either, so the named unit alone would + // coerce to i8 where classic gives i64. + cir.func @take_bitfield_unnamed_span(%arg0: !rec_UBitUnnamed) { + cir.return + } + + // CHECK: not yet implemented for type '!cir.union<"UBitUnnamed" + + // The same shape with a wider named unit, whose coercion happens to reach + // classic's i64 anyway once the union sizes its eightbyte. The rule cannot + // tell that apart from the case above, where the narrower unit coerces to + // i8, so this one is NYI as well. + cir.func @take_wide_bitfield_unnamed_span(%arg0: !rec_UWideBitUnnamed) { + cir.return + } + + // CHECK: not yet implemented for type '!cir.union<"UWideBitUnnamed" + // An empty member does not exempt a union from the spanning rule above: // as with take_short_storage_union, neither member reaches the union's // 4 declared bytes. _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
