https://github.com/adams381 created https://github.com/llvm/llvm-project/pull/216499
A class with a non-trivial destructor is caller-destroyed, so x86_64 passes it byref, meaning the callee works on the caller's storage rather than on a copy. The callee side already did that, but the call site did not. It copied the record into a fresh slot and passed that. The callee therefore mutated a copy nothing read, and the caller destroyed the stale original. For a class that owns a heap buffer, that is a double free. CIRGen already materializes the argument into a temporary it destroys after the call, and the call operand is a plain load of that temporary. The fix is to forward that alloca instead of a copy of its value. A byval argument keeps its fresh copy. An operand that is not a plain load of an alloca has no caller storage to forward, so it reports NYI rather than quietly copying. Assisted-by: Cursor / claude-opus-5 >From 48bf7b0ab34ec6148054cd4dbb542330fdf857a5 Mon Sep 17 00:00:00 2001 From: Adam Smith <[email protected]> Date: Sat, 15 Aug 2026 10:28:27 -0700 Subject: [PATCH] [CIR] Forward caller storage for byref call arguments A class with a non-trivial destructor is caller-destroyed, so x86_64 passes it byref, meaning the callee works on the caller's storage rather than on a copy. The callee side already did that, but the call site did not. It copied the record into a fresh slot and passed that. The callee therefore mutated a copy nothing read, and the caller destroyed the stale original. For a class that owns a heap buffer, that is a double free. CIRGen already materializes the argument into a temporary it destroys after the call, and the call operand is a plain load of that temporary. The fix is to forward that alloca instead of a copy of its value. A byval argument keeps its fresh copy. An operand that is not a plain load of an alloca has no caller storage to forward, so it reports NYI rather than quietly copying. Assisted-by: Cursor / claude-opus-5 --- .../TargetLowering/CIRABIRewriteContext.cpp | 121 ++++++++------ .../call-conv-lowering-x86_64-byref.cpp | 91 ++++++++++ .../abi-lowering/indirect-byref-nyi.cir | 155 ++++++++++++++++++ .../abi-lowering/indirect-byval.cir | 153 ++++++++++++++++- 4 files changed, 466 insertions(+), 54 deletions(-) create mode 100644 clang/test/CIR/CodeGen/call-conv-lowering-x86_64-byref.cpp create mode 100644 clang/test/CIR/Transforms/abi-lowering/indirect-byref-nyi.cir diff --git a/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp b/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp index cc79f8486b9f1..89ddbefa05195 100644 --- a/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp +++ b/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp @@ -23,11 +23,11 @@ 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. 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. +// the callee gets llvm.byref without the ownership attrs. At the call site +// byval copies into a fresh alloca while byref forwards the caller's storage. +// 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 @@ -119,11 +119,8 @@ buildNewArgTypes(ArrayRef<mlir::Type> oldArgTypes, newArgTypes.push_back(origTy); break; case ArgKind::Indirect: - // byval and byref both use a pointer wire type. The attribute - // distinction (llvm.byval vs llvm.byref) is applied in updateArgAttrs; - // the call-site rewrite guards against byref separately because passing - // a byref pointer from a CIR value requires the original alloca address, - // which the rewriter does not yet track. + // byval and byref both use a pointer wire type. The llvm.byval vs + // llvm.byref distinction is applied in updateArgAttrs. newArgTypes.push_back(cir::PointerType::get(origTy)); break; } @@ -225,8 +222,8 @@ mlir::ArrayAttr updateArgAttrs(mlir::MLIRContext *ctx, // llvm.noalias -- the copy is a fresh caller-allocated alloca that // no other pointer in the function can alias. Classic CodeGen // emits this when -fpass-by-value-is-noalias is set; here we - // emit it unconditionally because our call-site rewrite always - // produces a fresh alloca+store. + // emit it unconditionally because the byval call-site rewrite + // always produces a fresh alloca+store. mlir::Type pointeeTy = origArgTypes[oldIdx]; StringRef ownershipAttr = ac.byVal ? mlir::LLVM::LLVMDialect::getByValAttrName() @@ -410,6 +407,27 @@ void insertReturnCoercion(mlir::FunctionOpInterface funcOp, } } +/// A whole-record value's backing storage: the plain load that produced it and +/// the alloca that load read. Both fields are set or both are null. +struct WholeRecordSource { + cir::LoadOp load; + cir::AllocaOp alloca; +}; + +/// Look through \p recordVal to its backing alloca, or return a null +/// WholeRecordSource when it has none: a call result, a compound literal, a +/// load of a member of an enclosing record, or a load whose volatile or +/// memory-order semantics make the look-through observable. +static WholeRecordSource getWholeRecordSource(mlir::Value recordVal) { + cir::LoadOp load = recordVal.getDefiningOp<cir::LoadOp>(); + if (!load || load.getIsVolatile() || load.getMemOrder()) + return {}; + auto alloca = load.getAddr().getDefiningOp<cir::AllocaOp>(); + if (!alloca) + return {}; + return {load, alloca}; +} + /// Decompose a struct value into one scalar call argument per field of \p /// recTy, appending the field values to \p newArgs. When \p structVal is a /// plain (non-volatile, non-atomic) load straight from an alloca, read each @@ -426,21 +444,18 @@ emitStructFieldArgs(mlir::OpBuilder &builder, mlir::Location loc, mlir::Value structVal, cir::RecordType recTy, SmallVectorImpl<mlir::Value> &newArgs, SmallVectorImpl<cir::LoadOp> &replacedWholeLoads) { - cir::LoadOp wholeLoad = structVal.getDefiningOp<cir::LoadOp>(); - cir::AllocaOp srcAlloca; - if (wholeLoad && !wholeLoad.getIsVolatile() && !wholeLoad.getMemOrder()) - srcAlloca = wholeLoad.getAddr().getDefiningOp<cir::AllocaOp>(); + WholeRecordSource src = getWholeRecordSource(structVal); - if (srcAlloca) { + if (src.alloca) { mlir::OpBuilder::InsertionGuard guard(builder); - builder.setInsertionPoint(wholeLoad); + builder.setInsertionPoint(src.load); for (auto [f, fieldTy] : llvm::enumerate(recTy.getMembers())) { mlir::Type fieldPtrTy = cir::PointerType::get(fieldTy); mlir::Value fieldPtr = cir::GetMemberOp::create( - builder, loc, fieldPtrTy, srcAlloca, /*name=*/"", /*index=*/f); + builder, loc, fieldPtrTy, src.alloca, /*name=*/"", /*index=*/f); newArgs.push_back(cir::LoadOp::create(builder, loc, fieldPtr)); } - replacedWholeLoads.push_back(wholeLoad); + replacedWholeLoads.push_back(src.load); } else { for (unsigned f = 0; f < recTy.getNumElements(); ++f) newArgs.push_back( @@ -448,6 +463,17 @@ emitStructFieldArgs(mlir::OpBuilder &builder, mlir::Location loc, } } +/// Erase the whole-record loads a call-site rewrite read around, once the +/// original call (their remaining user) is gone. A single load can feed +/// several operands (e.g. after CSE merges identical loads), so dedupe before +/// erasing to avoid touching a freed op twice. +static void eraseDeadWholeRecordLoads(ArrayRef<cir::LoadOp> loads) { + SmallPtrSet<mlir::Operation *, 4> erased; + for (cir::LoadOp wholeLoad : loads) + if (erased.insert(wholeLoad).second && wholeLoad.use_empty()) + wholeLoad->erase(); +} + /// 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 arg, @@ -1153,9 +1179,9 @@ CIRABIRewriteContext::rewriteCallSite(mlir::Operation *callOp, mlir::ValueRange argOperands = call.getArgOperands(); newArgs.reserve(argOperands.size()); - // Whole-struct loads replaced by direct member loads for Expand operands. - // They can only be erased once the original call (their remaining user) is - // gone, so collect them and erase the dead ones at the end. + // Whole-record loads the rewrite reads around: replaced by direct member + // loads for Expand and Direct+canFlatten operands, or by the alloca itself + // for byref operands. SmallVector<cir::LoadOp> replacedWholeLoads; // Capture original arg types before building newArgs (byval slots change @@ -1204,21 +1230,30 @@ CIRABIRewriteContext::rewriteCallSite(mlir::Operation *callOp, dl); newArgs.push_back(arg); } else if (ac.kind == ArgKind::Indirect) { - // byval and byref: allocate a stack slot, copy the value in, and pass - // the pointer. The alloca+store pattern is identical for both; the - // attribute distinction (llvm.byval vs llvm.byref) is applied by - // updateArgAttrs. byref does not receive llvm.noalias or llvm.noundef - // because it does not assert exclusive ownership of the storage. - mlir::Type argTy = arg.getType(); - auto ptrTy = cir::PointerType::get(argTy); - uint64_t align = ac.indirectAlign.value(); - StringRef slotName = ac.byVal ? "byval" : "byref"; - auto slot = cir::AllocaOp::create(builder, call.getLoc(), ptrTy, - builder.getStringAttr(slotName), - builder.getI64IntegerAttr(align)); + // byval hands the callee its own copy. byref must name the caller's + // storage instead: CIRGen materializes the argument into a temporary it + // destroys after the call and emits the operand's load immediately + // before that call, so forwarding the alloca hands the callee the object + // the caller destroys, with nothing able to write it in between. + if (!ac.byVal) { + WholeRecordSource src = getWholeRecordSource(arg); + if (!src.alloca) + return call->emitOpError() + << "byref argument that is not a load of an alloca is not yet " + "implemented in CallConvLowering"; + assert(src.alloca.getAlignment() >= ac.indirectAlign.value() && + "llvm.align on a byref argument must not overstate the " + "forwarded slot"); + newArgs.push_back(src.alloca); + replacedWholeLoads.push_back(src.load); + continue; + } + auto ptrTy = cir::PointerType::get(arg.getType()); + auto slot = cir::AllocaOp::create( + builder, call.getLoc(), ptrTy, builder.getStringAttr("byval"), + builder.getI64IntegerAttr(ac.indirectAlign.value())); cir::StoreOp::create(builder, call.getLoc(), arg, slot); - arg = slot; - newArgs.push_back(arg); + newArgs.push_back(slot); } else { newArgs.push_back(arg); } @@ -1235,6 +1270,7 @@ CIRABIRewriteContext::rewriteCallSite(mlir::Operation *callOp, if (fc.returnInfo.kind == ArgKind::Indirect && hasResult) { rewriteIndirectReturnCall(call, fc, newArgs, origRetTy, origCallArgTypes, builder); + eraseDeadWholeRecordLoads(replacedWholeLoads); return mlir::success(); } @@ -1301,16 +1337,7 @@ CIRABIRewriteContext::rewriteCallSite(mlir::Operation *callOp, } call->erase(); - - // Now that the original call is gone, drop any whole-struct loads whose - // members we read directly from the source alloca, if nothing else uses - // them. A single load can feed several Expand operands (e.g. after CSE - // merges identical loads), so dedupe before erasing to avoid touching a - // freed op twice. - SmallPtrSet<mlir::Operation *, 4> erased; - for (cir::LoadOp wholeLoad : replacedWholeLoads) - if (erased.insert(wholeLoad).second && wholeLoad.use_empty()) - wholeLoad->erase(); + eraseDeadWholeRecordLoads(replacedWholeLoads); return mlir::success(); } diff --git a/clang/test/CIR/CodeGen/call-conv-lowering-x86_64-byref.cpp b/clang/test/CIR/CodeGen/call-conv-lowering-x86_64-byref.cpp new file mode 100644 index 0000000000000..b98f870604644 --- /dev/null +++ b/clang/test/CIR/CodeGen/call-conv-lowering-x86_64-byref.cpp @@ -0,0 +1,91 @@ +// 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 -emit-llvm %s -o %t-cir.ll +// RUN: FileCheck --check-prefixes=LLVM,LLVM-CIR --input-file=%t-cir.ll %s +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll +// RUN: FileCheck --check-prefixes=LLVM,OGCG --input-file=%t.ll %s + +struct WithDtor { + int x; + ~WithDtor(); +}; + +struct Big { + long a, b, c, d; +}; + +void takeByref(WithDtor t); +void takeTwoByref(WithDtor a, WithDtor b); +void takeByval(Big b); + +// The callee must receive the temporary the caller destroys, not a copy of it. +void callByref() { + WithDtor t; + takeByref(t); +} + +// CIR-LABEL: cir.func {{.*}}@_Z9callByrefv +// CIR: %[[T:.*]] = cir.alloca "t" align(4) : !cir.ptr<!rec_WithDtor> +// CIR: %[[TMP:.*]] = cir.alloca "agg.tmp0" align(4) : !cir.ptr<!rec_WithDtor> +// CIR: cir.copy %[[T]] align(4) to %[[TMP]] align(4) : !cir.ptr<!rec_WithDtor> +// CIR-NOT: cir.alloca "byref" +// CIR-NOT: cir.load +// CIR: cir.call @_Z9takeByref8WithDtor(%[[TMP]]) : (!cir.ptr<!rec_WithDtor> {llvm.align = 4 : i64, llvm.byref = !rec_WithDtor}) -> () +// CIR: cir.call @_ZN8WithDtorD1Ev(%[[TMP]]) +// CIR: cir.call @_ZN8WithDtorD1Ev(%[[T]]) + +// LLVM-LABEL: define dso_local void @_Z9callByrefv() +// LLVM: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %[[TMP:[^,]+]], ptr align 4 %[[T:[^,]+]], i64 4, i1 false) +// CIR marks the byref argument and drops the ownership and dereferenceability +// attrs classic emits, and classic adds dead_on_return on the destructor calls. +// LLVM-CIR: call void @_Z9takeByref8WithDtor(ptr byref(%struct.WithDtor) align 4 %[[TMP]]) +// LLVM-CIR: call void @_ZN8WithDtorD1Ev(ptr noundef nonnull align 4 dereferenceable(4) %[[TMP]]) +// LLVM-CIR: call void @_ZN8WithDtorD1Ev(ptr noundef nonnull align 4 dereferenceable(4) %[[T]]) +// OGCG: call void @_Z9takeByref8WithDtor(ptr nofree noundef align 4 dereferenceable(4) %[[TMP]]) +// OGCG: call void @_ZN8WithDtorD1Ev(ptr noundef nonnull align 4 dead_on_return(4) dereferenceable(4) %[[TMP]]) +// OGCG: call void @_ZN8WithDtorD1Ev(ptr noundef nonnull align 4 dead_on_return(4) dereferenceable(4) %[[T]]) + +// Each byref argument forwards its own temporary. +void callTwoByref() { + WithDtor a, b; + takeTwoByref(a, b); +} + +// CIR-LABEL: cir.func {{.*}}@_Z12callTwoByrefv +// CIR: %[[TMP_A:.*]] = cir.alloca "agg.tmp0" align(4) : !cir.ptr<!rec_WithDtor> +// CIR: %[[TMP_B:.*]] = cir.alloca "agg.tmp1" align(4) : !cir.ptr<!rec_WithDtor> +// CIR-NOT: cir.alloca "byref" +// CIR-NOT: cir.load +// CIR: cir.call @_Z12takeTwoByref8WithDtorS_(%[[TMP_A]], %[[TMP_B]]) : (!cir.ptr<!rec_WithDtor> {llvm.align = 4 : i64, llvm.byref = !rec_WithDtor}, !cir.ptr<!rec_WithDtor> {llvm.align = 4 : i64, llvm.byref = !rec_WithDtor}) -> () +// CIR: cir.call @_ZN8WithDtorD1Ev(%[[TMP_B]]) +// CIR: cir.call @_ZN8WithDtorD1Ev(%[[TMP_A]]) + +// LLVM-LABEL: define dso_local void @_Z12callTwoByrefv() +// LLVM: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %[[TMP_A:[^,]+]], ptr align 4 %{{[^,]+}}, i64 4, i1 false) +// LLVM: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %[[TMP_B:[^,]+]], ptr align 4 %{{[^,]+}}, i64 4, i1 false) +// LLVM-CIR: call void @_Z12takeTwoByref8WithDtorS_(ptr byref(%struct.WithDtor) align 4 %[[TMP_A]], ptr byref(%struct.WithDtor) align 4 %[[TMP_B]]) +// LLVM-CIR: call void @_ZN8WithDtorD1Ev(ptr noundef nonnull align 4 dereferenceable(4) %[[TMP_B]]) +// LLVM-CIR: call void @_ZN8WithDtorD1Ev(ptr noundef nonnull align 4 dereferenceable(4) %[[TMP_A]]) +// OGCG: call void @_Z12takeTwoByref8WithDtorS_(ptr nofree noundef align 4 dereferenceable(4) %[[TMP_A]], ptr nofree noundef align 4 dereferenceable(4) %[[TMP_B]]) +// OGCG: call void @_ZN8WithDtorD1Ev(ptr noundef nonnull align 4 dead_on_return(4) dereferenceable(4) %[[TMP_B]]) +// OGCG: call void @_ZN8WithDtorD1Ev(ptr noundef nonnull align 4 dead_on_return(4) dereferenceable(4) %[[TMP_A]]) + +// byval keeps the fresh copy the callee owns. +void callByval() { + Big b; + takeByval(b); +} + +// CIR-LABEL: cir.func {{.*}}@_Z9callByvalv +// CIR: %[[TMP:.*]] = cir.alloca "agg.tmp0" align(8) : !cir.ptr<!rec_Big> +// CIR: %[[V:.*]] = cir.load align(8) %[[TMP]] : !cir.ptr<!rec_Big>, !rec_Big +// CIR: %[[SLOT:.*]] = cir.alloca "byval" align(8) : !cir.ptr<!rec_Big> +// CIR: cir.store %[[V]], %[[SLOT]] : !rec_Big, !cir.ptr<!rec_Big> +// CIR: cir.call @_Z9takeByval3Big(%[[SLOT]]) : (!cir.ptr<!rec_Big> {llvm.align = 8 : i64, llvm.byval = !rec_Big, llvm.noalias, llvm.noundef}) -> () + +// LLVM-LABEL: define dso_local void @_Z9callByvalv() +// LLVM: call void @llvm.memcpy.p0.p0.i64(ptr align 8 %[[TMP:[^,]+]], ptr align 8 %{{[^,]+}}, i64 32, i1 false) +// LLVM-CIR: %[[V:.*]] = load %struct.Big, ptr %[[TMP]], align 8 +// LLVM-CIR: store %struct.Big %[[V]], ptr %[[SLOT:.*]], align 8 +// LLVM-CIR: call void @_Z9takeByval3Big(ptr noalias noundef byval(%struct.Big) align 8 %[[SLOT]]) +// OGCG: call void @_Z9takeByval3Big(ptr noundef byval(%struct.Big) align 8 %[[TMP]]) diff --git a/clang/test/CIR/Transforms/abi-lowering/indirect-byref-nyi.cir b/clang/test/CIR/Transforms/abi-lowering/indirect-byref-nyi.cir new file mode 100644 index 0000000000000..96ce07edd4a3c --- /dev/null +++ b/clang/test/CIR/Transforms/abi-lowering/indirect-byref-nyi.cir @@ -0,0 +1,155 @@ +// 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}> + +#byref_arg = { + return = { kind = "direct" }, + args = [ { kind = "indirect", indirect_align = 8, byval = false } ] +} + +#passthrough = { + return = { kind = "direct" }, + args = [ ] +} + +module attributes { + dlti.dl_spec = #dlti.dl_spec< + #dlti.dl_entry<i64, dense<64>: vector<2xi64>>> +} { + + cir.func private @takes_big_byref(%arg0: !rec_Big) + attributes { test_classify = #byref_arg } + + // A record value with no storage behind it: nothing to forward. + cir.func @caller_byref_value(%novalue: !rec_Big) + attributes { test_classify = #passthrough } { + cir.call @takes_big_byref(%novalue) : (!rec_Big) -> () + cir.return + } + + // CHECK: error: 'cir.call' op byref argument that is not a load of an alloca is not yet implemented in CallConvLowering + // CHECK-NEXT: cir.call @takes_big_byref(%novalue) + +} + +// ----- + +!s64i = !cir.int<s, 64> +!rec_Big = !cir.struct<"Big" {data !s64i, data !s64i, data !s64i, data !s64i}> + +#byref_arg = { + return = { kind = "direct" }, + args = [ { kind = "indirect", indirect_align = 8, byval = false } ] +} + +#passthrough = { + return = { kind = "direct" }, + args = [ ] +} + +module attributes { + dlti.dl_spec = #dlti.dl_spec< + #dlti.dl_entry<i64, dense<64>: vector<2xi64>>> +} { + + cir.func private @takes_big_byref(%arg0: !rec_Big) + attributes { test_classify = #byref_arg } + + // A volatile load is an observable access the look-through would delete. + cir.func @caller_byref_volatile(%s: !rec_Big) + attributes { test_classify = #passthrough } { + %tmp = cir.alloca "agg.tmp" align(8) : !cir.ptr<!rec_Big> + cir.store %s, %tmp : !rec_Big, !cir.ptr<!rec_Big> + %volatileload = cir.load volatile %tmp : !cir.ptr<!rec_Big>, !rec_Big + cir.call @takes_big_byref(%volatileload) : (!rec_Big) -> () + cir.return + } + + // CHECK: error: 'cir.call' op byref argument that is not a load of an alloca is not yet implemented in CallConvLowering + // CHECK-NEXT: cir.call @takes_big_byref(%volatileload) + +} + +// ----- + +!s64i = !cir.int<s, 64> +!rec_Big = !cir.struct<"Big" {data !s64i, data !s64i, data !s64i, data !s64i}> + +#byref_arg = { + return = { kind = "direct" }, + args = [ { kind = "indirect", indirect_align = 8, byval = false } ] +} + +#passthrough = { + return = { kind = "direct" }, + args = [ ] +} + +module attributes { + dlti.dl_spec = #dlti.dl_spec< + #dlti.dl_entry<i64, dense<64>: vector<2xi64>>> +} { + + cir.func private @takes_big_byref(%arg0: !rec_Big) + attributes { test_classify = #byref_arg } + + // An atomic load carries ordering the look-through would drop. + cir.func @caller_byref_atomic(%s: !rec_Big) + attributes { test_classify = #passthrough } { + %tmp = cir.alloca "agg.tmp" align(8) : !cir.ptr<!rec_Big> + cir.store %s, %tmp : !rec_Big, !cir.ptr<!rec_Big> + %atomicload = cir.load align(8) atomic(seq_cst) %tmp + : !cir.ptr<!rec_Big>, !rec_Big + cir.call @takes_big_byref(%atomicload) : (!rec_Big) -> () + cir.return + } + + // CHECK: error: 'cir.call' op byref argument that is not a load of an alloca is not yet implemented in CallConvLowering + // CHECK-NEXT: cir.call @takes_big_byref(%atomicload) + +} + +// ----- + +!s64i = !cir.int<s, 64> +!rec_Big = !cir.struct<"Big" {data !s64i, data !s64i, data !s64i, data !s64i}> +!rec_Outer = !cir.struct<"Outer" {data !rec_Big}> + +#byref_arg = { + return = { kind = "direct" }, + args = [ { kind = "indirect", indirect_align = 8, byval = false } ] +} + +#passthrough = { + return = { kind = "direct" }, + args = [ ] +} + +module attributes { + dlti.dl_spec = #dlti.dl_spec< + #dlti.dl_entry<i64, dense<64>: vector<2xi64>>> +} { + + cir.func private @takes_big_byref(%arg0: !rec_Big) + attributes { test_classify = #byref_arg } + + // A load of a member of an enclosing record is not the temporary the caller + // destroys, so it is reported rather than forwarded. + cir.func @caller_byref_member(%s: !rec_Outer) + attributes { test_classify = #passthrough } { + %tmp = cir.alloca "outer" align(8) : !cir.ptr<!rec_Outer> + cir.store %s, %tmp : !rec_Outer, !cir.ptr<!rec_Outer> + %member = cir.get_member %tmp[0] {name = "inner"} + : !cir.ptr<!rec_Outer> -> !cir.ptr<!rec_Big> + %memberload = cir.load %member : !cir.ptr<!rec_Big>, !rec_Big + cir.call @takes_big_byref(%memberload) : (!rec_Big) -> () + cir.return + } + + // CHECK: error: 'cir.call' op byref argument that is not a load of an alloca is not yet implemented in CallConvLowering + // CHECK-NEXT: cir.call @takes_big_byref(%memberload) + +} diff --git a/clang/test/CIR/Transforms/abi-lowering/indirect-byval.cir b/clang/test/CIR/Transforms/abi-lowering/indirect-byval.cir index 07a951e8054f7..deb6192a35e09 100644 --- a/clang/test/CIR/Transforms/abi-lowering/indirect-byval.cir +++ b/clang/test/CIR/Transforms/abi-lowering/indirect-byval.cir @@ -38,6 +38,24 @@ args = [ { kind = "indirect", indirect_align = 8, byval = false } ] } +#sret_byref = { + return = { kind = "indirect", indirect_align = 8 }, + args = [ { kind = "indirect", indirect_align = 8, byval = false } ] +} + +#mixed_byref = { + return = { kind = "direct" }, + args = [ { kind = "direct" }, + { kind = "indirect", indirect_align = 8, byval = false }, + { kind = "direct" } ] +} + +#two_byref_args = { + return = { kind = "direct" }, + args = [ { kind = "indirect", indirect_align = 8, byval = false }, + { kind = "indirect", indirect_align = 8, byval = false } ] +} + #passthrough = { return = { kind = "direct" }, args = [ ] @@ -278,19 +296,140 @@ module attributes { // CHECK-NOT: llvm.noalias // CHECK-NOT: llvm.noundef - // byref call site: same alloca+store pattern as byval, but the pointer - // carries llvm.byref (and no llvm.noalias / llvm.noundef since byref - // does not assert exclusive ownership of the storage). + // byref call site: the caller's temporary is passed rather than a copy, and + // the load that fed the call is left dead and erased. cir.func @caller_byref(%s: !rec_Big) -> !rec_Big attributes { test_classify = #passthrough } { - %r = cir.call @takes_big_byref(%s) : (!rec_Big) -> !rec_Big + %tmp = cir.alloca "agg.tmp" align(8) : !cir.ptr<!rec_Big> + cir.store %s, %tmp : !rec_Big, !cir.ptr<!rec_Big> + %v = cir.load %tmp : !cir.ptr<!rec_Big>, !rec_Big + %r = cir.call @takes_big_byref(%v) : (!rec_Big) -> !rec_Big cir.return %r : !rec_Big } // CHECK: cir.func{{.*}} @caller_byref(%[[S:.*]]: !rec_Big) -> !rec_Big - // CHECK: %[[SLOT:.*]] = cir.alloca "byref" align(8) : !cir.ptr<!rec_Big> - // CHECK-NEXT: cir.store %[[S]], %[[SLOT]] : !rec_Big, !cir.ptr<!rec_Big> - // CHECK-NEXT: %{{.*}} = cir.call @takes_big_byref(%[[SLOT]]) : + // CHECK: %[[TMP:.*]] = cir.alloca "agg.tmp" align(8) : !cir.ptr<!rec_Big> + // CHECK-NEXT: cir.store %[[S]], %[[TMP]] : !rec_Big, !cir.ptr<!rec_Big> + // CHECK-NOT: cir.alloca "byref" + // CHECK-NOT: cir.load + // CHECK: %{{.*}} = cir.call @takes_big_byref(%[[TMP]]) : + // CHECK-SAME: llvm.byref = !rec_Big + // CHECK-NOT: llvm.noalias + // CHECK-NOT: llvm.noundef + + // The whole-record load survives when a reader other than the call needs it. + cir.func @caller_byref_load_reused(%s: !rec_Big) -> !rec_Big + attributes { test_classify = #passthrough } { + %tmp = cir.alloca "agg.tmp" align(8) : !cir.ptr<!rec_Big> + cir.store %s, %tmp : !rec_Big, !cir.ptr<!rec_Big> + %v = cir.load %tmp : !cir.ptr<!rec_Big>, !rec_Big + %r = cir.call @takes_big_byref(%v) : (!rec_Big) -> !rec_Big + cir.return %v : !rec_Big + } + + // CHECK: cir.func{{.*}} @caller_byref_load_reused + // CHECK: %[[TMP:.*]] = cir.alloca "agg.tmp" align(8) : !cir.ptr<!rec_Big> + // CHECK: %[[V:.*]] = cir.load %[[TMP]] : !cir.ptr<!rec_Big>, !rec_Big + // CHECK-NOT: cir.alloca "byref" + // CHECK: %{{.*}} = cir.call @takes_big_byref(%[[TMP]]) : + // CHECK-SAME: llvm.byref = !rec_Big + // CHECK: cir.return %[[V]] : !rec_Big + + // CIRGen emits the load immediately before the call, so nothing can write + // the temporary in between. With a store in between the callee sees the + // stored value, since byref passes the object and not the loaded snapshot. + cir.func @caller_byref_store_between(%s: !rec_Big, %t: !rec_Big) -> !rec_Big + attributes { test_classify = #passthrough } { + %tmp = cir.alloca "agg.tmp" align(8) : !cir.ptr<!rec_Big> + cir.store %s, %tmp : !rec_Big, !cir.ptr<!rec_Big> + %v = cir.load %tmp : !cir.ptr<!rec_Big>, !rec_Big + cir.store %t, %tmp : !rec_Big, !cir.ptr<!rec_Big> + %r = cir.call @takes_big_byref(%v) : (!rec_Big) -> !rec_Big + cir.return %r : !rec_Big + } + + // CHECK: cir.func{{.*}} @caller_byref_store_between(%[[S:.*]]: !rec_Big, %[[T:.*]]: !rec_Big) + // CHECK: %[[TMP:.*]] = cir.alloca "agg.tmp" align(8) : !cir.ptr<!rec_Big> + // CHECK-NEXT: cir.store %[[S]], %[[TMP]] : !rec_Big, !cir.ptr<!rec_Big> + // CHECK-NEXT: cir.store %[[T]], %[[TMP]] : !rec_Big, !cir.ptr<!rec_Big> + // CHECK-NOT: cir.load + // CHECK-NOT: cir.alloca "byref" + // CHECK: %{{.*}} = cir.call @takes_big_byref(%[[TMP]]) : + // CHECK-SAME: llvm.byref = !rec_Big + + // A byref argument between two Direct ones keeps its operand slot, so the + // forwarded temporary lands at index 1 and its neighbours are untouched. + cir.func private @takes_mixed_byref(%a: !s32i, %b: !rec_Big, %c: !s32i) + attributes { test_classify = #mixed_byref } + + cir.func @caller_mixed_byref(%x: !s32i, %s: !rec_Big) + attributes { test_classify = #passthrough } { + %tmp = cir.alloca "agg.tmp" align(8) : !cir.ptr<!rec_Big> + cir.store %s, %tmp : !rec_Big, !cir.ptr<!rec_Big> + %v = cir.load %tmp : !cir.ptr<!rec_Big>, !rec_Big + cir.call @takes_mixed_byref(%x, %v, %x) : (!s32i, !rec_Big, !s32i) -> () + cir.return + } + + // CHECK: cir.func{{.*}} @caller_mixed_byref(%[[X:.*]]: !s32i + // CHECK: %[[TMP:.*]] = cir.alloca "agg.tmp" align(8) : !cir.ptr<!rec_Big> + // CHECK-NOT: cir.alloca "byref" + // CHECK: cir.call @takes_mixed_byref(%[[X]], %[[TMP]], %[[X]]) : + // CHECK-SAME: llvm.byref = !rec_Big + + // One load feeding two byref operands: the alloca is forwarded to both and + // the shared load is erased once. + cir.func private @takes_two_byref(%a: !rec_Big, %b: !rec_Big) + attributes { test_classify = #two_byref_args } + + cir.func @caller_two_byref_one_load(%s: !rec_Big) + attributes { test_classify = #passthrough } { + %tmp = cir.alloca "agg.tmp" align(8) : !cir.ptr<!rec_Big> + cir.store %s, %tmp : !rec_Big, !cir.ptr<!rec_Big> + %v = cir.load %tmp : !cir.ptr<!rec_Big>, !rec_Big + cir.call @takes_two_byref(%v, %v) : (!rec_Big, !rec_Big) -> () + cir.return + } + + // CHECK: cir.func{{.*}} @caller_two_byref_one_load + // CHECK: %[[TMP:.*]] = cir.alloca "agg.tmp" align(8) : !cir.ptr<!rec_Big> + // CHECK-NEXT: cir.store %{{.*}}, %[[TMP]] : !rec_Big, !cir.ptr<!rec_Big> + // CHECK-NOT: cir.load + // CHECK-NOT: cir.alloca "byref" + // CHECK: cir.call @takes_two_byref(%[[TMP]], %[[TMP]]) : + // CHECK-SAME: llvm.byref = !rec_Big + + // sret return plus a byref argument: the sret slot is prepended at operand 0, + // the forwarded temporary follows it, and the dead load is erased on the + // early-return sret path too. + cir.func @byref_and_sret(%arg0: !rec_Big) -> !rec_Big + attributes { test_classify = #sret_byref } { + %0 = cir.alloca "arg0" align(8) init : !cir.ptr<!rec_Big> + cir.store %arg0, %0 : !rec_Big, !cir.ptr<!rec_Big> + %1 = cir.alloca "__retval" align(8) : !cir.ptr<!rec_Big> + %z = cir.const #cir.zero : !rec_Big + cir.store %z, %1 : !rec_Big, !cir.ptr<!rec_Big> + %2 = cir.load %1 : !cir.ptr<!rec_Big>, !rec_Big + cir.return %2 : !rec_Big + } + + cir.func @caller_byref_and_sret(%s: !rec_Big) -> !rec_Big + attributes { test_classify = #passthrough } { + %tmp = cir.alloca "agg.tmp" align(8) : !cir.ptr<!rec_Big> + cir.store %s, %tmp : !rec_Big, !cir.ptr<!rec_Big> + %v = cir.load %tmp : !cir.ptr<!rec_Big>, !rec_Big + %r = cir.call @byref_and_sret(%v) : (!rec_Big) -> !rec_Big + cir.return %r : !rec_Big + } + + // CHECK: cir.func{{.*}} @caller_byref_and_sret + // CHECK: %[[TMP:.*]] = cir.alloca "agg.tmp" align(8) : !cir.ptr<!rec_Big> + // CHECK-NEXT: cir.store %{{.*}}, %[[TMP]] : !rec_Big, !cir.ptr<!rec_Big> + // CHECK-NOT: cir.alloca "byref" + // CHECK-NOT: cir.load + // CHECK: %[[RET:.*]] = cir.alloca "sret" align(8) : !cir.ptr<!rec_Big> + // CHECK: cir.call @byref_and_sret(%[[RET]], %[[TMP]]) : + // CHECK-SAME: llvm.sret = !rec_Big // CHECK-SAME: llvm.byref = !rec_Big // CHECK-NOT: llvm.noalias // CHECK-NOT: llvm.noundef _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
