https://github.com/adams381 created https://github.com/llvm/llvm-project/pull/224977
Return null from findParamSpill when a non-byval indirect parameter's only use is not a store into an alloca it names directly, instead of asserting that shape. Both callers already handled a null result, so an unmatched parameter now reaches the diagnostic the forwarding call already has for an operand that does not name the caller's storage. >From 5ec4adb3c0e04bb8f940d427567504110d2b77c9 Mon Sep 17 00:00:00 2001 From: Adam Smith <[email protected]> Date: Sun, 20 Sep 2026 16:17:20 -0700 Subject: [PATCH] [CIR] Match the parameter spill shape instead of asserting it Return null from findParamSpill when a non-byval indirect parameter's only use is not a store into an alloca it names directly, instead of asserting that shape. Both callers already handled a null result, so an unmatched parameter now reaches the diagnostic the forwarding call already has for an operand that does not name the caller's storage. Assisted-by: Cursor / claude-opus-5 --- .../TargetLowering/CIRABIRewriteContext.cpp | 28 ++++---- ...ering-x86_64-non-byval-param-spill-nyi.cpp | 30 ++++++++ .../non-byval-param-spill-nyi.cir | 71 +++++++++++++++++++ 3 files changed, 116 insertions(+), 13 deletions(-) create mode 100644 clang/test/CIR/CodeGen/call-conv-lowering-x86_64-non-byval-param-spill-nyi.cpp create mode 100644 clang/test/CIR/Transforms/abi-lowering/non-byval-param-spill-nyi.cir diff --git a/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp b/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp index 371fa67922872..1debf88aabfe3 100644 --- a/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp +++ b/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp @@ -558,22 +558,24 @@ static void eraseDeadRecordLoads(ArrayRef<cir::LoadOp> loads) { load->erase(); } -/// The store that spills non-byval indirect parameter \p blockArg, and the -/// slot it spills into. CIRGen spills every by-value parameter into a local -/// alloca with a single store before any other use, and this pass runs on that -/// CIRGen output before any alloca-promoting or splitting pass, so the block -/// argument has exactly that one use. Both results are null when DCE already -/// removed a dead spill. +/// The store that spills non-byval indirect parameter \p blockArg into a local +/// alloca it names directly, and that alloca. Both are null unless the block +/// argument's only use is such a store. CIRGen emits the spill as that only +/// use, but an earlier pass can add more: CIRSimplify replaces each read of +/// the const slot a const-qualified parameter is spilled to with the stored +/// value, so the block argument then feeds the spill and every one of those +/// readers. static std::pair<cir::StoreOp, cir::AllocaOp> findParamSpill(mlir::BlockArgument blockArg) { - if (blockArg.use_empty()) + if (!blockArg.hasOneUse()) return {}; - assert(blockArg.hasOneUse() && - "non-byval arg must have exactly one use (the CIRGen param spill)"); - auto store = cast<cir::StoreOp>(*blockArg.user_begin()); - assert(store.getValue() == blockArg && - "non-byval arg's use must be the value operand of its store"); - return {store, cast<cir::AllocaOp>(store.getAddr().getDefiningOp())}; + auto store = dyn_cast<cir::StoreOp>(*blockArg.user_begin()); + if (!store || store.getValue() != blockArg) + return {}; + auto slot = dyn_cast_or_null<cir::AllocaOp>(store.getAddr().getDefiningOp()); + if (!slot) + return {}; + return {store, slot}; } /// For each Direct arg with a coerced type, change the block argument's type diff --git a/clang/test/CIR/CodeGen/call-conv-lowering-x86_64-non-byval-param-spill-nyi.cpp b/clang/test/CIR/CodeGen/call-conv-lowering-x86_64-non-byval-param-spill-nyi.cpp new file mode 100644 index 0000000000000..2f73140d20a94 --- /dev/null +++ b/clang/test/CIR/CodeGen/call-conv-lowering-x86_64-non-byval-param-spill-nyi.cpp @@ -0,0 +1,30 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -O1 -fclangir -emit-cir \ +// RUN: %s -o /dev/null -verify + +// -O1 is load-bearing twice: Impl's key function is not defined here, so the +// vtable and its thunk are only emitted when optimizing, and CIRSimplify, +// which folds the read of the const slot, only runs then. + +struct NonTrivial { + ~NonTrivial(); +}; + +struct HasVirtual { + virtual void h(); +}; + +struct Middle : HasVirtual {}; + +struct Renderer { + virtual void draw(NonTrivial); +}; + +// Renderer sits at a non-zero offset in Impl, so overriding draw needs a +// this-adjusting thunk. +struct Impl : Middle, Renderer { + // expected-error@+2 {{does not name the caller's storage}} + // expected-note@+1 {{see current operation}} + void draw(const NonTrivial); +}; + +void emit() { new Impl; } diff --git a/clang/test/CIR/Transforms/abi-lowering/non-byval-param-spill-nyi.cir b/clang/test/CIR/Transforms/abi-lowering/non-byval-param-spill-nyi.cir new file mode 100644 index 0000000000000..9b3cd37a8a957 --- /dev/null +++ b/clang/test/CIR/Transforms/abi-lowering/non-byval-param-spill-nyi.cir @@ -0,0 +1,71 @@ +// RUN: not cir-opt %s -split-input-file \ +// RUN: -cir-call-conv-lowering="classification-attr=test_classify" \ +// RUN: 2>&1 | FileCheck %s + +!s64i = !cir.int<s, 64> +!rec_Big = !cir.struct<"Big" {data !s64i, data !s64i, data !s64i, data !s64i}> + +#non_byval_arg = { + return = { kind = "direct" }, + args = [ { kind = "indirect", indirect_align = 8, byval = false } ] +} + +module attributes { + dlti.dl_spec = #dlti.dl_spec< + #dlti.dl_entry<i64, dense<64>: vector<2xi64>>> +} { + + cir.func private @takes_big(%arg0: !rec_Big) + attributes { test_classify = #non_byval_arg } + + // A non-byval parameter with no spill at all: its only use is the + // forwarding call, so there is no load to look through and no slot to point + // the callee at. + cir.func @forwards_unspilled(%unspilled: !rec_Big) + attributes { test_classify = #non_byval_arg } { + cir.call @takes_big(%unspilled) : (!rec_Big) -> () + cir.return + } + + // CHECK: error: 'cir.call' op non-byval indirect argument that + // CHECK-SAME: does not name the caller's storage is not yet + // CHECK-SAME: implemented in CallConvLowering + // CHECK-NEXT: cir.call @takes_big(%unspilled) + +} + +// ----- + +!s64i = !cir.int<s, 64> +!rec_Big = !cir.struct<"Big" {data !s64i, data !s64i, data !s64i, data !s64i}> + +#non_byval_arg = { + return = { kind = "direct" }, + args = [ { kind = "indirect", indirect_align = 8, byval = false } ] +} + +module attributes { + dlti.dl_spec = #dlti.dl_spec< + #dlti.dl_entry<i64, dense<64>: vector<2xi64>>> +} { + + cir.func private @takes_big(%arg0: !rec_Big) + attributes { test_classify = #non_byval_arg } + + // A parameter both spilled and forwarded, as CIRSimplify leaves it after + // folding a read of a const parameter's slot: the spill is one of two uses + // rather than the only one. + cir.func @spills_and_forwards(%spilled: !rec_Big) + attributes { test_classify = #non_byval_arg } { + %slot = cir.alloca "spilled" align(8) init const : !cir.ptr<!rec_Big> + cir.store %spilled, %slot : !rec_Big, !cir.ptr<!rec_Big> + cir.call @takes_big(%spilled) : (!rec_Big) -> () + cir.return + } + + // CHECK: error: 'cir.call' op non-byval indirect argument that + // CHECK-SAME: does not name the caller's storage is not yet + // CHECK-SAME: implemented in CallConvLowering + // CHECK-NEXT: cir.call @takes_big(%spilled) + +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
