[clang] [CIR] Rewire byref args to the incoming pointer (PR #210836)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 20 15:52:32 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clangir
Author: adams381
<details>
<summary>Changes</summary>
The CallConvLowering body rewrite for Indirect arguments treats byval and
byref the same way: it inserts a cir.load at function entry and reroutes body
uses to the loaded value. For byref — a non-trivially-copyable type passed by
pointer — that entry load is a byte-copy, so the callee works on a local copy
instead of the caller's storage. That breaks types whose representation embeds
self-referential pointers: libstdc++'s SSO std::string keeps _M_p pointing at
its own _M_local_buf, and a byte-copy leaves the copy's _M_p aliasing the
source's buffer.
This mirrors the sret return fix in insertSRetStores. For byref only, rewire
the CIRGen param-slot alloca to the incoming pointer and drop the spill store,
so the body operates on the caller's storage in place. byval keeps the
load-at-entry copy, which is correct there.
The rewrite is exercised through the classification-injection driver in
clang/test/CIR/Transforms/abi-lowering/indirect-byval.cir, covering a byref
field access on the incoming pointer and a byval control case that retains the
copy. The call site still copies a value operand into a fresh alloca before
passing it by reference; forwarding existing storage in place is left for a
follow-up.
---
Full diff: https://github.com/llvm/llvm-project/pull/210836.diff
2 Files Affected:
- (modified) clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp (+55-16)
- (modified) clang/test/CIR/Transforms/abi-lowering/indirect-byval.cir (+61-5)
``````````diff
diff --git a/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp b/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp
index 9fd1835243deb..b21e762895331 100644
--- a/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp
+++ b/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp
@@ -23,7 +23,10 @@ using namespace mlir::abi;
// For byval (ArgClassification::byVal == true) the callee gets
// llvm.byval + llvm.noalias + llvm.noundef; for byref (byVal == false)
// the callee gets llvm.byref without the ownership attrs. Both pass
-// through an alloca+store at the call site.
+// through an alloca+store at the call site. At the callee, byval loads
+// the incoming pointer (a local copy), while byref rewires the CIRGen
+// param-slot alloca to the incoming pointer so the body mutates the
+// caller's storage in place.
//
// For Expand, the single struct argument is replaced by N scalar arguments
// (one per field). At the callee, the N field block arguments are stored
@@ -437,9 +440,12 @@ emitStructFieldArgs(mlir::OpBuilder &builder, mlir::Location loc,
/// For each Direct arg with a coerced type, change the block argument's type
/// to the coerced type and insert a coercion at function entry that maps it
-/// back to the original type for body uses. For each Indirect (byval/byref)
-/// arg, change the block argument's type to a pointer and insert a load at
-/// entry so the body sees the original value type. For each Expand arg,
+/// back to the original type for body uses. For each Indirect byval arg,
+/// change the block argument's type to a pointer and insert a load at entry
+/// so the body sees a local copy of the original value type. For each
+/// Indirect byref arg, change the block argument to a pointer and rewire the
+/// CIRGen param-slot alloca to that pointer (no entry load / byte-copy) so
+/// the body operates on the caller's storage in place. For each Expand arg,
/// replace the single struct block argument with N scalar block arguments (one
/// per field) and store each field directly into the parameter's own alloca
/// (the CIRGen spill slot), erasing the original whole-struct store.
@@ -619,19 +625,52 @@ void insertArgCoercion(mlir::FunctionOpInterface funcOp,
// to adapted (now of the original type != the alloca's pointee type).
blockArg.replaceAllUsesExcept(adapted, coercionOps);
} else if (ac.kind == ArgKind::Indirect) {
- // byval and byref: the wire type is !cir.ptr<T>. Change the block arg
- // to the pointer type and insert a load so the body sees the original
- // T. The body transformation is the same for both; the distinction
- // between byval (llvm.byval) and byref (llvm.byref) is in the arg
- // attributes applied by updateArgAttrs.
- mlir::Type origTy = blockArg.getType();
- auto ptrTy = cir::PointerType::get(origTy);
- blockArg.setType(ptrTy);
+ // byval and byref share a !cir.ptr<T> wire type; the llvm.byval vs
+ // llvm.byref distinction is in the attrs applied by updateArgAttrs.
+ // Body lowering differs: byval copies into the callee (load at entry),
+ // while byref must operate on the caller's storage in place.
+ auto ptrTy = cir::PointerType::get(blockArg.getType());
+
+ if (!ac.byVal) {
+ // byref: CIRGen spills every by-value parameter into a local alloca
+ // with a single store before any other use. Rewire that alloca to
+ // the incoming pointer and drop the store, mirroring insertSRetStores
+ // for non-trivially-copyable aggregates (e.g. libstdc++ SSO
+ // std::string, where a byte-copy would leave `_M_p` aliasing the
+ // source's `_M_local_buf`). DCE may have removed a dead spill;
+ // tolerate that by only retyping the block argument.
+ cir::StoreOp paramStore;
+ cir::AllocaOp destAlloca;
+ if (!blockArg.use_empty()) {
+ assert(blockArg.hasOneUse() &&
+ "byref arg must have exactly one use (the CIRGen param "
+ "spill)");
+ paramStore = cast<cir::StoreOp>(*blockArg.user_begin());
+ assert(paramStore.getValue() == blockArg &&
+ "byref arg's use must be the value operand of its store");
+ destAlloca =
+ cast<cir::AllocaOp>(paramStore.getAddr().getDefiningOp());
+ }
- builder.setInsertionPointToStart(&entry);
- auto loadOp = cir::LoadOp::create(builder, funcOp.getLoc(), blockArg);
- SmallPtrSet<mlir::Operation *, 1> loadOps = {loadOp};
- blockArg.replaceAllUsesExcept(loadOp.getResult(), loadOps);
+ if (paramStore)
+ paramStore->erase();
+
+ blockArg.setType(ptrTy);
+
+ if (destAlloca) {
+ destAlloca.getResult().replaceAllUsesWith(blockArg);
+ destAlloca->erase();
+ }
+ } else {
+ // byval: load the incoming pointer so the body sees a T value (and
+ // any CIRGen param-slot store becomes a local copy of that value).
+ blockArg.setType(ptrTy);
+
+ builder.setInsertionPointToStart(&entry);
+ auto loadOp = cir::LoadOp::create(builder, funcOp.getLoc(), blockArg);
+ SmallPtrSet<mlir::Operation *, 1> loadOps = {loadOp};
+ blockArg.replaceAllUsesExcept(loadOp.getResult(), loadOps);
+ }
}
// Ignore, Extend, and Direct-without-coerce need no block-level changes.
diff --git a/clang/test/CIR/Transforms/abi-lowering/indirect-byval.cir b/clang/test/CIR/Transforms/abi-lowering/indirect-byval.cir
index f0a8572403fe9..887d8f5b0d7cb 100644
--- a/clang/test/CIR/Transforms/abi-lowering/indirect-byval.cir
+++ b/clang/test/CIR/Transforms/abi-lowering/indirect-byval.cir
@@ -196,13 +196,15 @@ module attributes {
// CHECK: cir.store %{{.*}}, %{{.*}} : !rec_Big, !cir.ptr<!rec_Big>
// CHECK: cir.return
- // byref callee definition: the parameter gets llvm.byref instead of
- // llvm.byval. noalias and noundef are NOT added (byref passes the
- // original, which may alias and may carry padding). The body
- // transformation (ptr + load at entry) is identical to byval.
+ // byref callee: llvm.byref without noalias/noundef. CIRGen spills the
+ // by-value param into a local alloca; the rewriter rewires that alloca to
+ // the incoming pointer (no entry load / byte-copy).
cir.func @takes_big_byref(%arg0: !rec_Big) -> !rec_Big
attributes { test_classify = #byref_arg } {
- cir.return %arg0 : !rec_Big
+ %0 = cir.alloca "arg0" align(8) init : !cir.ptr<!rec_Big>
+ cir.store %arg0, %0 : !rec_Big, !cir.ptr<!rec_Big>
+ %1 = cir.load %0 : !cir.ptr<!rec_Big>, !rec_Big
+ cir.return %1 : !rec_Big
}
// CHECK: cir.func{{.*}} @takes_big_byref(%[[PTR:.*]]: !cir.ptr<!rec_Big>
@@ -210,9 +212,63 @@ module attributes {
// CHECK-SAME: llvm.byref = !rec_Big
// CHECK-NOT: llvm.noalias
// CHECK-NOT: llvm.noundef
+ // CHECK-NOT: cir.alloca
// CHECK: %[[L:.*]] = cir.load %[[PTR]] : !cir.ptr<!rec_Big>, !rec_Big
// CHECK-NEXT: cir.return %[[L]] : !rec_Big
+ // byref field access must use the incoming pointer in place — no local
+ // alloca and no byte-copy of the non-trivially-copyable aggregate.
+ cir.func @takes_big_byref_field(%arg0: !rec_Big) -> !s64i
+ attributes { test_classify = #byref_arg } {
+ %0 = cir.alloca "arg0" align(8) init : !cir.ptr<!rec_Big>
+ cir.store %arg0, %0 : !rec_Big, !cir.ptr<!rec_Big>
+ %1 = cir.get_member %0[0] {name = "a"} : !cir.ptr<!rec_Big> -> !cir.ptr<!s64i>
+ %2 = cir.load %1 : !cir.ptr<!s64i>, !s64i
+ cir.return %2 : !s64i
+ }
+
+ // CHECK: cir.func{{.*}} @takes_big_byref_field(%[[PTR:.*]]: !cir.ptr<!rec_Big>
+ // CHECK-SAME: llvm.byref = !rec_Big
+ // CHECK-NOT: cir.alloca
+ // CHECK: %[[M:.*]] = cir.get_member %[[PTR]][0] {name = "a"} : !cir.ptr<!rec_Big> -> !cir.ptr<!s64i>
+ // CHECK-NEXT: %[[V:.*]] = cir.load %[[M]] : !cir.ptr<!s64i>, !s64i
+ // CHECK-NEXT: cir.return %[[V]] : !s64i
+
+ // byval with the same CIRGen spill keeps a local copy: load the byval
+ // pointer at entry and store into the param-slot alloca.
+ cir.func @takes_big_byval_field(%arg0: !rec_Big) -> !s64i
+ attributes { test_classify = #byval_arg } {
+ %0 = cir.alloca "arg0" align(8) init : !cir.ptr<!rec_Big>
+ cir.store %arg0, %0 : !rec_Big, !cir.ptr<!rec_Big>
+ %1 = cir.get_member %0[0] {name = "a"} : !cir.ptr<!rec_Big> -> !cir.ptr<!s64i>
+ %2 = cir.load %1 : !cir.ptr<!s64i>, !s64i
+ cir.return %2 : !s64i
+ }
+
+ // CHECK: cir.func{{.*}} @takes_big_byval_field(%[[PTR:.*]]: !cir.ptr<!rec_Big>
+ // CHECK-SAME: llvm.byval = !rec_Big
+ // CHECK: %[[LOADED:.*]] = cir.load %[[PTR]] : !cir.ptr<!rec_Big>, !rec_Big
+ // CHECK: %[[SLOT:.*]] = cir.alloca "arg0" align(8) init : !cir.ptr<!rec_Big>
+ // CHECK: cir.store %[[LOADED]], %[[SLOT]] : !rec_Big, !cir.ptr<!rec_Big>
+ // CHECK: %[[M:.*]] = cir.get_member %[[SLOT]][0] {name = "a"} : !cir.ptr<!rec_Big> -> !cir.ptr<!s64i>
+ // CHECK: %[[V:.*]] = cir.load %[[M]] : !cir.ptr<!s64i>, !s64i
+ // CHECK: cir.return %[[V]] : !s64i
+
+ // byref with the param spill removed (e.g. by DCE): the block arg has no
+ // use, so the rewriter only retypes it to a pointer and emits nothing else.
+ cir.func @takes_big_byref_unused(%arg0: !rec_Big) -> !s32i
+ attributes { test_classify = #byref_arg } {
+ %r = cir.const #cir.int<0> : !s32i
+ cir.return %r : !s32i
+ }
+
+ // CHECK: cir.func{{.*}} @takes_big_byref_unused(%{{.*}}: !cir.ptr<!rec_Big>
+ // CHECK-SAME: llvm.byref = !rec_Big
+ // CHECK-NOT: cir.alloca
+ // CHECK-NOT: cir.load
+ // CHECK: %[[R:.*]] = cir.const #cir.int<0> : !s32i
+ // CHECK-NEXT: cir.return %[[R]] : !s32i
+
// byref forward declaration: signature gets llvm.byref, no body.
cir.func private @takes_big_byref_decl(%arg0: !rec_Big) -> !rec_Big
attributes { test_classify = #byref_arg }
``````````
</details>
https://github.com/llvm/llvm-project/pull/210836
More information about the cfe-commits
mailing list