https://github.com/adams381 updated https://github.com/llvm/llvm-project/pull/218786
>From 0c706f807ffb35bb7c7572cfea637821167e1f55 Mon Sep 17 00:00:00 2001 From: Adam Smith <[email protected]> Date: Tue, 25 Aug 2026 14:13:44 -0700 Subject: [PATCH 1/3] [CIR] Skip ABI classification for an incomplete-record declaration A `cir.func` declaration whose signature carries an incomplete record by value caused the pass to fail the whole module. C++ requires a complete type at any call or definition, so only a declaration can carry this shape, and no translation unit anywhere can ever call or define it with real argument data. Classic CodeGen skips full ABI lowering rather than attempting one. We now leave such a declaration unclassified and match classic. Assisted-by: Cursor / claude-opus-5 --- .../Transforms/CallConvLoweringPass.cpp | 23 ++++++++ .../abi-lowering/x86_64-aggregate-nyi.cir | 10 ++++ .../x86_64-incomplete-record-declaration.cir | 54 +++++++++++++++++++ 3 files changed, 87 insertions(+) create mode 100644 clang/test/CIR/Transforms/abi-lowering/x86_64-incomplete-record-declaration.cir diff --git a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp index 11c2a66dba7b5..99edfbe50efd3 100644 --- a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp +++ b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp @@ -136,6 +136,20 @@ static bool reachesNamedBitFieldUnit(mlir::Type ty) { return llvm::any_of(recTy.getMembers(), reachesNamedBitFieldUnit); } +/// Whether \p ty, or an aggregate member/element reached by value (never +/// through a pointer), is an incomplete record. Such a record has no known +/// layout, so no eightbyte classification can be built for it. Classic skips +/// full ABI lowering for a function type that depends on an incomplete tag type +/// rather than attempting one. +static bool hasIncompleteRecordByValue(mlir::Type ty) { + if (auto recTy = dyn_cast<cir::RecordType>(ty)) + return !recTy.isComplete() || + llvm::any_of(recTy.getMembers(), hasIncompleteRecordByValue); + if (auto arrTy = dyn_cast<cir::ArrayType>(ty)) + return hasIncompleteRecordByValue(arrTy.getElementType()); + return false; +} + /// The CIR types the x86_64 bridge handles. Scalars: an integer up to 128 /// bits (including `_BitInt` and `__int128`), pointer, vtable pointer, bool, /// void, or any floating-point type. Aggregates: a complete struct or union @@ -839,6 +853,15 @@ void CallConvLoweringPass::runOnOperation() { llvm::MapVector<cir::FuncOp, FunctionClassification> classifications; bool anyFailed = false; moduleOp.walk([&](cir::FuncOp f) { + // C++ requires a complete type at any call or definition, so only a + // declaration can carry an incomplete-by-value parameter or return + // type, and no translation unit can ever call or define it with real + // argument data. Leave it unclassified, mirroring classic CodeGen. + cir::FuncType fnTy = f.getFunctionType(); + if (isX86 && f.isDeclaration() && + (hasIncompleteRecordByValue(fnTy.getReturnType()) || + llvm::any_of(fnTy.getInputs(), hasIncompleteRecordByValue))) + return; std::optional<FunctionClassification> fc; if (isX86) fc = classifyX86_64Function(f, dl, *x86TypeMapper, 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 f5f97a28fca95..08175318524a0 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 @@ -32,6 +32,7 @@ !rec_EBits = !cir.struct<"EBits" {empty !u32i}> !rec_HoldsEmptyBits = !cir.struct<"HoldsEmptyBits" {empty !rec_EBits, data !s32i}> !rec_LeadPad = !cir.struct<"LeadPad" {pad !cir.array<!u8i x 8>, data !s32i}> +!rec_Incomplete = !cir.struct<"Incomplete" incomplete> module attributes { dlti.dl_spec = #dlti.dl_spec< @@ -236,4 +237,13 @@ module attributes { } // CHECK: not yet implemented for type '!cir.struct<"NamedPlusZeroWidth" + + // A definition (unlike a declaration) is refused: no incomplete-by-value + // definition can arise from valid C++, so this stays a hard reject rather + // than the declaration-only skip in x86_64-incomplete-record-declaration.cir. + cir.func @take_incomplete_defined(%arg0: !rec_Incomplete) { + cir.return + } + + // CHECK: not yet implemented for type '!cir.struct<"Incomplete" incomplete>' } diff --git a/clang/test/CIR/Transforms/abi-lowering/x86_64-incomplete-record-declaration.cir b/clang/test/CIR/Transforms/abi-lowering/x86_64-incomplete-record-declaration.cir new file mode 100644 index 0000000000000..1710ab9567059 --- /dev/null +++ b/clang/test/CIR/Transforms/abi-lowering/x86_64-incomplete-record-declaration.cir @@ -0,0 +1,54 @@ +// RUN: cir-opt %s -cir-call-conv-lowering=target=x86_64 | FileCheck %s + +!s32i = !cir.int<s, 32> +!rec_Incomplete = !cir.struct<"Incomplete" incomplete> +!rec_Wraps = !cir.struct<"Wraps" {data !rec_Incomplete}> +!rec_Pair = !cir.struct<"Pair" {data !s32i, data !s32i}> + +module attributes { + dlti.dl_spec = #dlti.dl_spec< + #dlti.dl_entry<i32, dense<32>: vector<2xi64>>, + #dlti.dl_entry<i64, dense<64>: vector<2xi64>>> +} { + + // Only a declaration can name an incomplete-by-value parameter: a call or + // a definition requires the type to be complete. No layout means no + // eightbyte classification, but nothing here ever completes Incomplete + // either, so no call or definition of this exact signature can arise + // anywhere. Left unclassified and unrewritten. + cir.func private @take_incomplete(!rec_Incomplete) + + // CHECK: cir.func private @take_incomplete(!rec_Incomplete) + + // Same reasoning for an incomplete-by-value return type. + cir.func private @make_incomplete() -> !rec_Incomplete + + // CHECK: cir.func private @make_incomplete() -> !rec_Incomplete + + // The reject propagates through an enclosing struct and through an array + // element, the same as any other unsupported member. + cir.func private @take_wrapped(!rec_Wraps) + + // CHECK: cir.func private @take_wrapped(!rec_Wraps) + + cir.func private @take_incomplete_array(!cir.array<!rec_Incomplete x 4>) + + // CHECK: cir.func private @take_incomplete_array(!cir.array<!rec_Incomplete x 4>) + + // A classifiable function sharing the module with an unclassifiable + // declaration is still rewritten: skipping one function must not abort + // the whole pass the way a hard NYI error would. + cir.func @take_pair(%arg0: !rec_Pair) { + cir.return + } + + // CHECK: cir.func{{.*}} @take_pair(%arg0: !u64i) + + cir.func @call_pair(%arg0: !rec_Pair) { + cir.call @take_pair(%arg0) : (!rec_Pair) -> () + cir.return + } + + // CHECK: cir.func{{.*}} @call_pair(%arg0: !u64i) + // CHECK: cir.call @take_pair(%{{.*}}) : (!u64i) -> () +} >From 5492b189c91b9758550a66bb17534ebbe5e77862 Mon Sep 17 00:00:00 2001 From: Adam Smith <[email protected]> Date: Wed, 26 Aug 2026 09:05:39 -0700 Subject: [PATCH 2/3] [CIR] Address review comments, drop one more test's opt-out Update a couple of review comments to either stand alone instead of referencing what classic does or clarify what they say. The test case that was discussed already exists in the CIR lit suite. It was just opting out of the pass until this PR allowed it to run. Dropped the flag and confirmed the test passes with the pass on and no CHECK changes are needed. Assisted-by: Cursor / claude-opus-5 --- .../CIR/Dialect/Transforms/CallConvLoweringPass.cpp | 12 +++++------- .../CodeGen/vtable-nyi-nonconvertible-functype.cpp | 6 ++---- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp index 99edfbe50efd3..891a50c20de11 100644 --- a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp +++ b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp @@ -138,9 +138,7 @@ static bool reachesNamedBitFieldUnit(mlir::Type ty) { /// Whether \p ty, or an aggregate member/element reached by value (never /// through a pointer), is an incomplete record. Such a record has no known -/// layout, so no eightbyte classification can be built for it. Classic skips -/// full ABI lowering for a function type that depends on an incomplete tag type -/// rather than attempting one. +/// layout, so no eightbyte classification can be built for it. static bool hasIncompleteRecordByValue(mlir::Type ty) { if (auto recTy = dyn_cast<cir::RecordType>(ty)) return !recTy.isComplete() || @@ -853,10 +851,10 @@ void CallConvLoweringPass::runOnOperation() { llvm::MapVector<cir::FuncOp, FunctionClassification> classifications; bool anyFailed = false; moduleOp.walk([&](cir::FuncOp f) { - // C++ requires a complete type at any call or definition, so only a - // declaration can carry an incomplete-by-value parameter or return - // type, and no translation unit can ever call or define it with real - // argument data. Leave it unclassified, mirroring classic CodeGen. + // A complete type is required at any call or definition, so only a + // declaration can carry an incomplete-by-value parameter or return type, + // and no translation unit can ever call or define it with real argument + // data. Leave it unclassified. cir::FuncType fnTy = f.getFunctionType(); if (isX86 && f.isDeclaration() && (hasIncompleteRecordByValue(fnTy.getReturnType()) || diff --git a/clang/test/CIR/CodeGen/vtable-nyi-nonconvertible-functype.cpp b/clang/test/CIR/CodeGen/vtable-nyi-nonconvertible-functype.cpp index d1ca653f2de73..2675044179cb2 100644 --- a/clang/test/CIR/CodeGen/vtable-nyi-nonconvertible-functype.cpp +++ b/clang/test/CIR/CodeGen/vtable-nyi-nonconvertible-functype.cpp @@ -1,8 +1,6 @@ -// TODO(cir): drop -fno-clangir-call-conv-lowering once CallConvLowering -// supports padded, packed, and over-aligned record shapes. -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -fno-clangir-call-conv-lowering -emit-cir %s -o %t.cir +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir // RUN: FileCheck --check-prefix=CIR --input-file=%t.cir %s -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -fno-clangir-call-conv-lowering -emit-llvm %s -o %t-cir.ll +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o %t-cir.ll // RUN: FileCheck --check-prefix=LLVM,LLVMCIR --input-file=%t-cir.ll %s // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll // RUN: FileCheck --check-prefix=LLVM,OGCG --input-file=%t.ll %s >From aea3906c76154bc22e0938e3d7e24a12e4322b17 Mon Sep 17 00:00:00 2001 From: Adam Smith <[email protected]> Date: Thu, 27 Aug 2026 13:58:54 -0700 Subject: [PATCH 3/3] [CIR] Drop the isX86 gate on the incomplete-record skip Eli asked why the incomplete-record skip only applied to the x86_64 driver. The reasoning behind it, that only a declaration can carry this shape and no translation unit can ever call or define it, is a language rule, not a target one, so the gate had no real reason to be there. Added a case to declaration-rewrite.cir that injects an "ignore" classification for an incomplete-by-value parameter and checks the argument survives anyway. Assisted-by: Cursor / claude-opus-5 --- .../CIR/Dialect/Transforms/CallConvLoweringPass.cpp | 2 +- .../Transforms/abi-lowering/declaration-rewrite.cir | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp index 891a50c20de11..922aa6940c3b3 100644 --- a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp +++ b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp @@ -856,7 +856,7 @@ void CallConvLoweringPass::runOnOperation() { // and no translation unit can ever call or define it with real argument // data. Leave it unclassified. cir::FuncType fnTy = f.getFunctionType(); - if (isX86 && f.isDeclaration() && + if (f.isDeclaration() && (hasIncompleteRecordByValue(fnTy.getReturnType()) || llvm::any_of(fnTy.getInputs(), hasIncompleteRecordByValue))) return; diff --git a/clang/test/CIR/Transforms/abi-lowering/declaration-rewrite.cir b/clang/test/CIR/Transforms/abi-lowering/declaration-rewrite.cir index 2242ed65d74bc..1e6acf7e01dc1 100644 --- a/clang/test/CIR/Transforms/abi-lowering/declaration-rewrite.cir +++ b/clang/test/CIR/Transforms/abi-lowering/declaration-rewrite.cir @@ -6,12 +6,18 @@ // adaptation runs. !s32i = !cir.int<s, 32> +!rec_Incomplete = !cir.struct<"Incomplete" incomplete> #ignore_first_arg = { return = { kind = "direct" }, args = [ { kind = "ignore" }, { kind = "direct" } ] } +#ignore_only_arg = { + return = { kind = "direct" }, + args = [ { kind = "ignore" } ] +} + module attributes { dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<i32, dense<32>: vector<2xi64>>> } { @@ -31,4 +37,10 @@ module attributes { // CHECK: cir.func{{.*}} @caller(%arg0: !s32i) -> !s32i // CHECK: %[[R:.*]] = cir.call @ext_decl(%arg0) : (!s32i) -> !s32i + // "ignore" would drop the argument, but an incomplete one is left alone. + cir.func private @incomplete_decl(!rec_Incomplete) + attributes { test_classify = #ignore_only_arg } + + // CHECK: cir.func private @incomplete_decl(!rec_Incomplete) + } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
