[clang] [CIR] Accept unions in x86_64 calling-convention lowering (PR #214129)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 4 20:54:13 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Adam Smith (adams381)
<details>
<summary>Changes</summary>
Passing a union to a function does not work. The x86_64 bridge rejects every one, so the pass fails on any signature naming a union. Additionally, indirect arguments have their byval and sret alignment wrong, because `mapCIRType` asks DataLayout for it, and DataLayout only sees a record's members, never `__attribute__((aligned(N)))`.
Unions now go through the ABI library's union type, which puts every member at offset zero and sizes each eightbyte from the union rather than from a single member. The alignment comes from the record-layout metadata the AST already fills in, which fixes over-aligned structs too, since they share that lookup.
Assisted-by: Cursor / claude-opus-5
---
Patch is 39.85 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/214129.diff
7 Files Affected:
- (modified) clang/include/clang/CIR/Dialect/IR/CIRDialect.h (+5)
- (modified) clang/lib/CIR/Dialect/IR/CIRAttrs.cpp (+12-4)
- (modified) clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp (+79-44)
- (modified) clang/test/CIR/CodeGen/call-conv-lowering-x86_64.c (+84)
- (modified) clang/test/CIR/Transforms/abi-lowering/x86_64-aggregate-nyi.cir (+77-5)
- (added) clang/test/CIR/Transforms/abi-lowering/x86_64-union-coerce-shapes.cir (+80)
- (added) clang/test/CIR/Transforms/abi-lowering/x86_64-union.cir (+280)
``````````diff
diff --git a/clang/include/clang/CIR/Dialect/IR/CIRDialect.h b/clang/include/clang/CIR/Dialect/IR/CIRDialect.h
index c6f6c80206bfe..8cd70bf47ce5a 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIRDialect.h
+++ b/clang/include/clang/CIR/Dialect/IR/CIRDialect.h
@@ -84,6 +84,11 @@ class FenvOpTrait : public mlir::OpTrait::TraitBase<ConcreteType, FenvOpTrait> {
/// Look up the RecordLayoutAttr for a named record in the module's
/// cir.record_layouts dictionary. Asserts if the entry is missing.
RecordLayoutAttr getRecordLayout(mlir::ModuleOp module, mlir::StringAttr name);
+
+/// Same lookup as getRecordLayout, but returns a null attribute instead of
+/// asserting when the record has no layout entry.
+RecordLayoutAttr tryGetRecordLayout(mlir::ModuleOp module,
+ mlir::StringAttr name);
} // namespace cir
// TableGen'erated files for MLIR dialects require that a macro be defined when
diff --git a/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp b/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp
index 264e836718c81..74a3deb5b2508 100644
--- a/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp
@@ -920,12 +920,20 @@ LogicalResult DynamicCastInfoAttr::verify(
// RecordLayout lookup
//===----------------------------------------------------------------------===//
-RecordLayoutAttr cir::getRecordLayout(mlir::ModuleOp module,
- mlir::StringAttr name) {
+RecordLayoutAttr cir::tryGetRecordLayout(mlir::ModuleOp module,
+ mlir::StringAttr name) {
+ if (!name)
+ return {};
auto dict = module->getAttrOfType<mlir::DictionaryAttr>(
CIRDialect::getRecordLayoutsAttrName());
- assert(dict && "module missing cir.record_layouts attribute");
- auto attr = dict.getAs<RecordLayoutAttr>(name);
+ if (!dict)
+ return {};
+ return dict.getAs<RecordLayoutAttr>(name);
+}
+
+RecordLayoutAttr cir::getRecordLayout(mlir::ModuleOp module,
+ mlir::StringAttr name) {
+ RecordLayoutAttr attr = tryGetRecordLayout(module, name);
assert(attr && "record layout entry missing for named record");
return attr;
}
diff --git a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp
index 54c4487db6195..1baa145251bc9 100644
--- a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp
+++ b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp
@@ -68,10 +68,11 @@ namespace {
// 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 / bool /
-// f32 / f64 scalars and struct / array aggregates are handled. Unions,
-// `_Complex`, vectors, wider floats, and packed or padded records are reported
-// NYI by classifyX86_64Function so an unsupported signature fails the pass
-// instead of being misclassified.
+// f32 / f64 scalars and struct / union / array aggregates are handled.
+// `_Complex`, vectors, wider floats, packed or padded records, and a union no
+// member of which spans its declared size 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
@@ -79,27 +80,32 @@ namespace {
/// no layout entry (e.g. an anonymous struct) has no C++ non-trivial reason to
/// be forced to memory, so it defaults to can-pass-in-registers.
static bool recordCanPassInRegs(ModuleOp modOp, cir::RecordType recTy) {
- mlir::StringAttr name = recTy.getName();
- if (!name)
- return true;
- auto dict = modOp->getAttrOfType<DictionaryAttr>(
- cir::CIRDialect::getRecordLayoutsAttrName());
- if (!dict)
- return true;
- auto layout = dict.getAs<cir::RecordLayoutAttr>(name);
+ auto layout = cir::tryGetRecordLayout(modOp, recTy.getName());
if (!layout)
return true;
return layout.getArgPassingKind() == cir::ArgPassingKind::CanPassInRegs;
}
+/// A record's declared alignment, which the ABI uses for the byval and sret
+/// alignment of an indirect argument. DataLayout derives alignment from the
+/// members, so it cannot see `__attribute__((aligned(N)))`. The declared value
+/// comes from the module's record-layout metadata instead. CIRGen emits an
+/// entry for every record it names, so the computed fallback only serves
+/// hand-written CIR.
+static llvm::Align recordDeclaredAlign(ModuleOp modOp, cir::RecordType recTy,
+ const DataLayout &dl) {
+ auto layout = cir::tryGetRecordLayout(modOp, recTy.getName());
+ if (!layout)
+ return llvm::Align(dl.getTypeABIAlignment(recTy));
+ return llvm::Align(layout.getRecordAlign());
+}
+
/// The CIR types the x86_64 bridge handles. Scalars: an integer up to 128
/// bits (including `_BitInt` and `__int128`), pointer, bool, void, f32, or f64.
-/// Aggregates: a complete struct whose fields are all themselves supported, or
-/// an array of a supported element type. A `_BitInt` wider than 128 bits,
-/// unions, `_Complex`, vectors, wider floats, and packed or padded records are
-/// not handled and are reported NYI at the reject() choke point in
-/// classifyX86_64Function.
-static bool isSupportedType(mlir::Type ty) {
+/// Aggregates: a complete struct or union whose members are all themselves
+/// supported, or an array of a supported element type. Everything else is
+/// reported NYI at the reject() choke point in classifyX86_64Function.
+static bool isSupportedType(mlir::Type ty, const DataLayout &dl) {
// A pointer is only handled in the default address space (null) or an
// already-lowered target address space. A LangAddressSpaceAttr must be
// lowered before this pass, so reject it rather than silently dropping it.
@@ -124,21 +130,40 @@ static bool isSupportedType(mlir::Type ty) {
return intTy.getWidth() <= 64 || intTy.getWidth() == 128;
}
if (auto arrTy = dyn_cast<cir::ArrayType>(ty))
- return isSupportedType(arrTy.getElementType());
+ return isSupportedType(arrTy.getElementType(), dl);
if (auto recTy = dyn_cast<cir::RecordType>(ty)) {
- // Unions and packed / padded records each need classification this bridge
- // does not implement (a union widen fixup and pad-aware eightbyte
- // classification), so reject them here and report NYI rather than
- // misclassify. A zero-field record (a C empty struct) classifies as
- // Ignore and is dropped from the lowered signature. CIRGen lays out an
- // empty C++ class as a single padded byte, which the padded check rejects.
- // A real one-byte struct such as `{char[1]}` has a field and is not
- // padded, so it is classified normally.
- if (recTy.isUnion() || !recTy.isComplete() || recTy.getPacked() ||
- recTy.getPadded())
+ // An incomplete record has no layout to classify, and a packed one needs
+ // pad-aware eightbyte classification this bridge does not implement.
+ if (!recTy.isComplete() || recTy.getPacked())
return false;
+ if (recTy.isUnion()) {
+ // The classifier sizes a union's eightbytes from the union itself, which
+ // is only sound when some member spans that size. Short of that, the
+ // remaining bytes are either tail padding or the rest of a bitfield
+ // storage unit, and the CIR type cannot tell those apart even though
+ // classic CodeGen coerces them to i32 and i8 respectively.
+ llvm::ArrayRef<mlir::Type> members = recTy.getMembers();
+ uint64_t recordBits = dl.getTypeSizeInBits(recTy).getFixedValue();
+ if (members.empty()) {
+ // A member-less union is all padding, which classifies Ignore up to two
+ // eightbytes. Past that SysV says MEMORY regardless of content, and
+ // there is no member here to build the Indirect coercion from.
+ if (recordBits > 128)
+ return false;
+ } else {
+ auto spansRecord = [&](mlir::Type m) {
+ return dl.getTypeSizeInBits(m).getFixedValue() == recordBits;
+ };
+ if (!llvm::any_of(members, spansRecord))
+ return false;
+ }
+ } else if (recTy.getPadded()) {
+ // A struct's padding is a member the classifier would have to recognize
+ // as padding rather than data, which is not implemented.
+ return false;
+ }
return llvm::all_of(recTy.getMembers(),
- [](mlir::Type m) { return isSupportedType(m); });
+ [&](mlir::Type m) { return isSupportedType(m, dl); });
}
return false;
}
@@ -220,11 +245,28 @@ static const llvm::abi::Type *mapCIRType(mlir::Type type,
dl.getTypeSizeInBits(type).getFixedValue());
})
.Case([&](cir::RecordType recTy) -> const llvm::abi::Type * {
- // isSupportedType rejects unions, packed / padded, and empty-for-ABI
- // records, so this handles a plain struct: map each field at its
- // naturally-aligned offset.
+ llvm::abi::RecordFlags flags = llvm::abi::RecordFlags::None;
+ if (recordCanPassInRegs(modOp, recTy))
+ flags = flags | llvm::abi::RecordFlags::CanPassInRegisters;
+ llvm::TypeSize sizeBits = llvm::TypeSize::getFixed(
+ dl.getTypeSizeInBits(type).getFixedValue());
+ llvm::Align align = recordDeclaredAlign(modOp, recTy, dl);
SmallVector<llvm::abi::FieldInfo> fields;
fields.reserve(recTy.getMembers().size());
+
+ // The size passed here spans the tail padding, so an eightbyte covers
+ // 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)));
+ return tb.getUnionType(fields, sizeBits, align,
+ llvm::abi::StructPacking::Default, flags);
+ }
+
+ // isSupportedType rejects packed and padded structs, so every field
+ // here sits at its naturally-aligned offset.
uint64_t offsetBits = 0;
for (mlir::Type fieldTy : recTy.getMembers()) {
const llvm::abi::Type *mappedField =
@@ -234,16 +276,9 @@ static const llvm::abi::Type *mapCIRType(mlir::Type type,
fields.push_back(llvm::abi::FieldInfo(mappedField, offsetBits));
offsetBits += dl.getTypeSizeInBits(fieldTy).getFixedValue();
}
- llvm::abi::RecordFlags flags = llvm::abi::RecordFlags::None;
- if (recordCanPassInRegs(modOp, recTy))
- flags = flags | llvm::abi::RecordFlags::CanPassInRegisters;
- return tb.getRecordType(fields,
- llvm::TypeSize::getFixed(
- dl.getTypeSizeInBits(type).getFixedValue()),
- llvm::Align(dl.getTypeABIAlignment(type)),
- llvm::abi::StructPacking::Default,
- /*BaseClasses=*/{}, /*VirtualBaseClasses=*/{},
- flags);
+ return tb.getRecordType(
+ fields, sizeBits, align, llvm::abi::StructPacking::Default,
+ /*BaseClasses=*/{}, /*VirtualBaseClasses=*/{}, flags);
})
.Default([](mlir::Type) -> const llvm::abi::Type * {
llvm_unreachable(
@@ -388,7 +423,7 @@ static std::optional<FunctionClassification> classifyX86_64Signature(
bool voidRet = isa<cir::VoidType>(retCIR);
auto reject = [&](mlir::Type t) -> bool {
- if (isSupportedType(t))
+ if (isSupportedType(t, dl))
return false;
emitError()
<< "x86_64 calling-convention lowering not yet implemented for type "
diff --git a/clang/test/CIR/CodeGen/call-conv-lowering-x86_64.c b/clang/test/CIR/CodeGen/call-conv-lowering-x86_64.c
index 6931198a90c3d..8382b15bb5d9e 100644
--- a/clang/test/CIR/CodeGen/call-conv-lowering-x86_64.c
+++ b/clang/test/CIR/CodeGen/call-conv-lowering-x86_64.c
@@ -11,6 +11,11 @@ typedef struct { long a, b, c, d; } Big;
typedef struct { long a; double b; } IntSSE;
typedef struct { double a; double b; } SSE2;
typedef struct { } Empty;
+typedef union { int i; float f; } UIntFloat;
+typedef union { float f; float g; } UFloats;
+typedef union { int i; char c[8]; } UNarrowStorage;
+typedef union { char c[32]; } UBig;
+typedef union { char c[32]; } __attribute__((aligned(32))) UBigOverAligned;
// Narrow signed integer sign-extended in a register.
signed char ext_schar(signed char c) { return c; }
@@ -79,3 +84,82 @@ void take_big(Big b) { (void)b; }
// CIR: cir.func {{.*}}@take_big(%arg0: !cir.ptr<!rec_Big> {{.*}}llvm.byval = !rec_Big{{.*}})
// LLVM-CIR: define dso_local void @take_big(ptr noalias noundef byval(%struct.Big) align 8 %{{.+}})
// LLVM-OGCG: define dso_local void @take_big(ptr noundef byval(%struct.Big) align 8 %{{.+}})
+
+// Union members all start at offset zero, so a 4-byte union takes one INTEGER
+// eightbyte and coerces to i32.
+void take_union(UIntFloat u) { (void)u; }
+
+// CIR: cir.func {{.*}}@take_union(%arg0: !s32i{{.*}})
+// LLVM: define dso_local void @take_union(i32 %{{.+}})
+
+// A union of floats classifies SSE, so it coerces to a float register.
+void take_union_floats(UFloats u) { (void)u; }
+
+// CIR: cir.func {{.*}}@take_union_floats(%arg0: !cir.float{{.*}})
+// LLVM: define dso_local void @take_union_floats(float %{{.+}})
+
+// The union's highest-aligned member is the 4-byte int, but its size comes
+// from the 8-byte array, and the eightbyte is sized from the union.
+void take_union_narrow_storage(UNarrowStorage u) { (void)u; }
+
+// CIR: cir.func {{.*}}@take_union_narrow_storage(%arg0: !u64i{{.*}})
+// LLVM: define dso_local void @take_union_narrow_storage(i64 %{{.+}})
+
+// A coerced union return round-trips through the coercion type.
+UIntFloat ret_union(int a) { UIntFloat u; u.i = a; return u; }
+
+// CIR: cir.func {{.*}}@ret_union(%arg0: !s32i {{.*}}) -> !s32i
+// LLVM: define dso_local i32 @ret_union(i32 noundef %{{.+}})
+
+// A union too large for registers is passed byval, with the same noalias
+// divergence as a large struct.
+void take_union_big(UBig u) { (void)u; }
+
+// CIR: cir.func {{.*}}@take_union_big(%arg0: !cir.ptr<!rec_UBig> {{.*}}llvm.byval = !rec_UBig{{.*}})
+// LLVM-CIR: define dso_local void @take_union_big(ptr noalias noundef byval(%union.UBig) align 8 %{{.+}})
+// LLVM-OGCG: define dso_local void @take_union_big(ptr noundef byval(%union.UBig) align 8 %{{.+}})
+
+// The byval alignment follows the union's declared alignment, not the alignment
+// its members imply, which is 1 here.
+void take_union_big_over_aligned(UBigOverAligned u) { (void)u; }
+
+// CIR: cir.func {{.*}}@take_union_big_over_aligned(%arg0: !cir.ptr<!rec_UBigOverAligned> {{.*}}llvm.align = 32 : i64{{.*}}llvm.byval = !rec_UBigOverAligned{{.*}})
+// LLVM-CIR: define dso_local void @take_union_big_over_aligned(ptr noalias noundef byval(%union.UBigOverAligned) align 32 %{{.+}})
+// LLVM-OGCG: define dso_local void @take_union_big_over_aligned(ptr noundef byval(%union.UBigOverAligned) align 32 %{{.+}})
+
+void call_union(UIntFloat u) { take_union(u); }
+
+// CIR: cir.func {{.*}}@call_union(%arg0: !s32i
+// CIR: cir.call @take_union(%{{.+}}) : (!s32i) -> ()
+// LLVM: define dso_local void @call_union(i32 %{{.+}})
+// LLVM: call void @take_union(i32 %{{.+}})
+
+void call_union_big_over_aligned(UBigOverAligned u) {
+ take_union_big_over_aligned(u);
+}
+
+// CIR: cir.func {{.*}}@call_union_big_over_aligned(%arg0: !cir.ptr<!rec_UBigOverAligned> {{.*}}llvm.align = 32 : i64{{.*}})
+// CIR: cir.call @take_union_big_over_aligned(%{{.+}}) : (!cir.ptr<!rec_UBigOverAligned> {{.*}}llvm.align = 32 : i64{{.*}}) -> ()
+// LLVM-CIR: define dso_local void @call_union_big_over_aligned(ptr noalias noundef byval(%union.UBigOverAligned) align 32 %{{.+}})
+// LLVM-CIR: alloca %union.UBigOverAligned, i64 1, align 32
+// LLVM-CIR: call void @take_union_big_over_aligned(ptr noalias noundef byval(%union.UBigOverAligned) align 32 %{{.+}})
+// LLVM-OGCG: define dso_local void @call_union_big_over_aligned(ptr noundef byval(%union.UBigOverAligned) align 32 %{{.+}})
+// LLVM-OGCG: call void @take_union_big_over_aligned(ptr noundef byval(%union.UBigOverAligned) align 32 %{{.+}})
+
+// The declared alignment reaches the sret slot of an indirect return too, not
+// just a byval argument.
+UBigOverAligned ret_union_big_over_aligned(void);
+void call_ret_union_big_over_aligned(void) { (void)ret_union_big_over_aligned(); }
+
+// CIR: cir.func {{.*}}@ret_union_big_over_aligned(!cir.ptr<!rec_UBigOverAligned> {{.*}}llvm.align = 32 : i64{{.*}}llvm.sret = !rec_UBigOverAligned{{.*}})
+// LLVM: declare void @ret_union_big_over_aligned(ptr dead_on_unwind writable sret(%union.UBigOverAligned) align 32)
+
+// The same declared-alignment source feeds an over-aligned struct, since
+// mapCIRType's alignment lookup is on the shared record path, not a
+// union-specific one.
+typedef struct { char c[32]; } __attribute__((aligned(32))) SOverAligned;
+void take_struct_over_aligned(SOverAligned s) { (void)s; }
+
+// CIR: cir.func {{.*}}@take_struct_over_aligned(%arg0: !cir.ptr<!rec_SOverAligned> {{.*}}llvm.align = 32 : i64{{.*}}llvm.byval = !rec_SOverAligned{{.*}})
+// LLVM-CIR: define dso_local void @take_struct_over_aligned(ptr noalias noundef byval(%struct.SOverAligned) align 32 %{{.+}})
+// LLVM-OGCG: define dso_local void @take_struct_over_aligned(ptr noundef byval(%struct.SOverAligned) align 32 %{{.+}})
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 85f05fef9f96b..83d36ce22b970 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
@@ -1,10 +1,17 @@
// RUN: not cir-opt %s -cir-call-conv-lowering=target=x86_64 2>&1 | FileCheck %s
!s8i = !cir.int<s, 8>
+!s16i = !cir.int<s, 16>
!s32i = !cir.int<s, 32>
!u8i = !cir.int<u, 8>
-!u32i = !cir.int<u, 32>
-!rec_U = !cir.union<"U" {!s32i, !u32i}>
+!rec_UPacked = !cir.union<"UPacked" packed {!s32i, !cir.array<!s8i x 5>}, padding = {!u8i}>
+!rec_ULongDouble = !cir.union<"ULongDouble" {!cir.long_double<!cir.f80>, !s32i}>
+!rec_UFloats = !cir.union<"UFloats" {!cir.array<!cir.float x 2>, !cir.array<!cir.float x 2>}>
+!rec_UOverAligned = !cir.union<"UOverAligned" {!s32i}, padding = {!cir.array<!u8i x 12>}>
+!rec_UShortStorage = !cir.union<"UShortStorage" {!s16i, !cir.array<!s8i x 3>}, padding = {!cir.array<!u8i x 2>}>
+!rec_UByteBlobs = !cir.union<"UByteBlobs" {!u8i, !u8i}, padding = {!cir.array<!u8i x 3>}>
+!rec_SWrapsOverAligned = !cir.struct<"SWrapsOverAligned" {!cir.double, !rec_UOverAligned}>
+!rec_UEmptyLarge = !cir.union<"UEmptyLarge" {}, padding = {!cir.array<!u8i x 32>}>
!rec_P = !cir.struct<"P" packed {!s8i, !s32i}>
!rec_Ov = !cir.struct<"Ov" padded {!s32i, !cir.array<!u8i x 12>}>
!rec_E = !cir.struct<"E" padded {!u8i}>
@@ -19,12 +26,77 @@ module attributes {
#dlti.dl_entry<f64, dense<64>: vector<2xi64>>>
} {
- // A union is rejected: its register coercion needs a widen fixup.
- cir.func @take_union(%arg0: !rec_U) {
+ // A packed union is rejected for the same reason a packed struct is: its
+ // members no longer sit at their natural alignment.
+ cir.func @take_packed_union(%arg0: !rec_UPacked) {
cir.return
}
- // CHECK: not yet implemented for type '!cir.union<"U"
+ // CHECK: not yet implemented for type '!cir.union<"UPacked" packed
+
+ // A union member the bridge does not map keeps the whole union unsupported.
+ cir.func @take_union_long_double(%arg0: !rec_ULongDouble) {
+ cir.return
+ }
+
+ // CHECK: not yet implemented for type '!cir.union<"ULongDouble"
+
+ // A union whose highest-aligned member is an all-float array classifies to
+ // an SSE vector coerce this bridge does not represent, so it is reported NYI
+ // rather than passed unchanged.
+ cir.func @take_union_float_arrays(%arg0: !rec_UFloats) {
+ cir.return
+ }
+
+ // CHECK: not yet implemented for the ABI coercion of type '!cir.union<"UFloats...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/214129
More information about the cfe-commits
mailing list