[clang] 911ec69 - [CIR] Forward caller storage for byref call arguments (#216499)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 25 06:16:22 PDT 2026
Author: Adam Smith
Date: 2026-08-25T08:16:17-05:00
New Revision: 911ec697968461118767d96af811db934914ba0f
URL: https://github.com/llvm/llvm-project/commit/911ec697968461118767d96af811db934914ba0f
DIFF: https://github.com/llvm/llvm-project/commit/911ec697968461118767d96af811db934914ba0f.diff
LOG: [CIR] Forward caller storage for byref call arguments (#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
Added:
clang/test/CIR/CodeGen/call-conv-lowering-x86_64-byref.cpp
clang/test/CIR/Transforms/abi-lowering/indirect-byref-nyi.cir
Modified:
clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp
clang/test/CIR/Transforms/abi-lowering/indirect-byval.cir
Removed:
################################################################################
diff --git a/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp b/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp
index cc79f8486b9f1..8d4c3b19c213d 100644
--- a/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp
+++ b/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp
@@ -12,6 +12,7 @@
#include "mlir/IR/Dominance.h"
#include "clang/CIR/Dialect/IR/CIRAttrs.h"
#include "clang/CIR/Dialect/IR/CIRTypes.h"
+#include <utility>
using namespace cir;
using namespace mlir;
@@ -21,13 +22,20 @@ using namespace mlir::abi;
// Extend, Ignore, Indirect-return (sret), Indirect-argument (byval and
// byref), and Expand (struct flattening) classifications.
//
+// "byref" here is the llvm.byref case of an Indirect argument, not C++
+// pass-by-reference. A C++ reference parameter is already a pointer by the
+// time this classifier runs, so it classifies Direct. byref instead means
+// a by-value parameter whose type cannot be copied freely, because it has a
+// non-trivial copy constructor, move constructor, or destructor, so the ABI
+// passes it through a pointer instead of in registers.
+//
// 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 +127,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 pass a pointer. Which of the two it is shows up
+ // in the attributes updateArgAttrs applies, not in the type.
newArgTypes.push_back(cir::PointerType::get(origTy));
break;
}
@@ -225,8 +230,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,37 +415,51 @@ void insertReturnCoercion(mlir::FunctionOpInterface funcOp,
}
}
+/// If \p recordVal is a plain load of an alloca, return that alloca and the
+/// load. Return nulls otherwise: a volatile or atomic load has to keep its
+/// ordering, and a call result or a load of a member of a larger record has no
+/// alloca of its own.
+static std::pair<cir::AllocaOp, cir::LoadOp>
+getWholeRecordSource(mlir::Value recordVal) {
+ cir::LoadOp load = recordVal.getDefiningOp<cir::LoadOp>();
+ if (!load || load.getIsVolatile() || load.getMemOrder())
+ return {};
+ // TODO: look through cir.cast ops where isAllocaPreservingCast() is true,
+ // mirroring Address::getUnderlyingAllocaOp() (CodeGen/Address.h), so an
+ // address-space-cast alloca is still found here once offload targets need it.
+ auto alloca = load.getAddr().getDefiningOp<cir::AllocaOp>();
+ if (!alloca)
+ return {};
+ return {alloca, load};
+}
+
/// 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
/// field with cir.get_member + cir.load from that alloca, emitted at the
/// original load's position so they observe the same memory state, and record
-/// the now-dead whole-struct load in \p replacedWholeLoads for later erasure.
+/// the now-dead whole-struct load in \p deadRecordLoads for later erasure.
/// Otherwise (a call result, compound literal, or qualified load) extract each
/// field from the value with cir.extract_member. Loading the members from the
/// alloca rather than extracting from a whole-struct value keeps the result in
/// a form SROA can promote (it does not reason about extractvalue). Shared by
/// the Expand and Direct+canFlatten argument paths.
-static void
-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>();
+static void emitStructFieldArgs(mlir::OpBuilder &builder, mlir::Location loc,
+ mlir::Value structVal, cir::RecordType recTy,
+ SmallVectorImpl<mlir::Value> &newArgs,
+ SmallVectorImpl<cir::LoadOp> &deadRecordLoads) {
+ auto [srcAlloca, srcLoad] = getWholeRecordSource(structVal);
if (srcAlloca) {
mlir::OpBuilder::InsertionGuard guard(builder);
- builder.setInsertionPoint(wholeLoad);
+ builder.setInsertionPoint(srcLoad);
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);
newArgs.push_back(cir::LoadOp::create(builder, loc, fieldPtr));
}
- replacedWholeLoads.push_back(wholeLoad);
+ deadRecordLoads.push_back(srcLoad);
} else {
for (unsigned f = 0; f < recTy.getNumElements(); ++f)
newArgs.push_back(
@@ -448,6 +467,18 @@ emitStructFieldArgs(mlir::OpBuilder &builder, mlir::Location loc,
}
}
+/// Erase the loads that a rewritten call left unused. The old call must
+/// already be erased, since until then it still counts as a user. One load can
+/// feed two operands of the same call, as in f(s, s), so \p loads can hold the
+/// same load twice. A load that another op still reads is left alone.
+static void eraseDeadRecordLoads(ArrayRef<cir::LoadOp> loads) {
+ llvm::SmallSetVector<mlir::Operation *, 4> uniqueLoads(llvm::from_range,
+ loads);
+ for (mlir::Operation *load : uniqueLoads)
+ if (load->use_empty())
+ load->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,10 +1184,10 @@ 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.
- SmallVector<cir::LoadOp> replacedWholeLoads;
+ // Loads that the new call leaves unused: Expand and Direct+canFlatten read
+ // the fields out of the source alloca, and byref passes the alloca itself.
+ // The old call still uses them, so erase them only after it is gone.
+ SmallVector<cir::LoadOp> deadRecordLoads;
// Capture original arg types before building newArgs (byval slots change
// the wire argument from T to !cir.ptr<T>, so we save the pre-rewrite
@@ -1188,7 +1219,7 @@ CIRABIRewriteContext::rewriteCallSite(mlir::Operation *callOp,
}
} else {
emitStructFieldArgs(builder, call.getLoc(), arg, flatTy, newArgs,
- replacedWholeLoads);
+ deadRecordLoads);
}
} else if (ac.kind == ArgKind::Expand) {
// Decompose the struct value into its constituent scalar fields and
@@ -1197,28 +1228,37 @@ CIRABIRewriteContext::rewriteCallSite(mlir::Operation *callOp,
assert(recTy.isStruct() &&
"Expand classification requires a struct type, not a union");
emitStructFieldArgs(builder, call.getLoc(), arg, recTy, newArgs,
- replacedWholeLoads);
+ deadRecordLoads);
} else if (ac.kind == ArgKind::Direct && ac.coercedType &&
arg.getType() != ac.coercedType) {
arg = emitCoercion(builder, call.getLoc(), ac.coercedType, arg, slotBlock,
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) {
+ auto [srcAlloca, srcLoad] = getWholeRecordSource(arg);
+ if (!srcAlloca)
+ return call->emitOpError()
+ << "byref argument that is not a load of an alloca is not yet "
+ "implemented in CallConvLowering";
+ assert(srcAlloca.getAlignment() >= ac.indirectAlign.value() &&
+ "llvm.align on a byref argument must not overstate the "
+ "forwarded slot");
+ newArgs.push_back(srcAlloca);
+ deadRecordLoads.push_back(srcLoad);
+ 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 +1275,7 @@ CIRABIRewriteContext::rewriteCallSite(mlir::Operation *callOp,
if (fc.returnInfo.kind == ArgKind::Indirect && hasResult) {
rewriteIndirectReturnCall(call, fc, newArgs, origRetTy, origCallArgTypes,
builder);
+ eraseDeadRecordLoads(deadRecordLoads);
return mlir::success();
}
@@ -1301,16 +1342,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();
+ eraseDeadRecordLoads(deadRecordLoads);
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..26b1187cc16d3
--- /dev/null
+++ b/clang/test/CIR/CodeGen/call-conv-lowering-x86_64-byref.cpp
@@ -0,0 +1,117 @@
+// 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;
+};
+
+struct WithCopyCtor {
+ int x;
+ WithCopyCtor();
+ WithCopyCtor(const WithCopyCtor &);
+};
+
+void takeByref(WithDtor t);
+void takeTwoByref(WithDtor a, WithDtor b);
+void takeByval(Big b);
+void takeCopyCtorByref(WithCopyCtor c);
+
+// 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.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.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]])
+
+// A non-trivial copy constructor also classifies byref: the constructor call
+// populates the forwarded temporary directly, with no load in between.
+void callCopyCtorByref() {
+ WithCopyCtor c;
+ takeCopyCtorByref(c);
+}
+
+// CIR-LABEL: cir.func {{.*}}@_Z17callCopyCtorByrefv
+// CIR: %[[C:.*]] = cir.alloca "c" align(4) init : !cir.ptr<!rec_WithCopyCtor>
+// CIR: %[[TMP:.*]] = cir.alloca "agg.tmp0" align(4) : !cir.ptr<!rec_WithCopyCtor>
+// CIR: cir.call @_ZN12WithCopyCtorC1Ev(%[[C]])
+// CIR: cir.call @_ZN12WithCopyCtorC1ERKS_(%[[TMP]], %[[C]])
+// CIR-NOT: cir.load
+// CIR: cir.call @_Z17takeCopyCtorByref12WithCopyCtor(%[[TMP]]) : (!cir.ptr<!rec_WithCopyCtor> {llvm.align = 4 : i64, llvm.byref = !rec_WithCopyCtor}) -> ()
+
+// LLVM-LABEL: define dso_local void @_Z17callCopyCtorByrefv()
+// LLVM: call void @_ZN12WithCopyCtorC1Ev(ptr noundef nonnull align 4 dereferenceable(4) %[[C:[^)]+]])
+// LLVM: call void @_ZN12WithCopyCtorC1ERKS_(ptr noundef nonnull align 4 dereferenceable(4) %[[TMP:[^,]+]], ptr noundef nonnull align 4 dereferenceable(4) %[[C]])
+// LLVM-CIR: call void @_Z17takeCopyCtorByref12WithCopyCtor(ptr byref(%struct.WithCopyCtor) align 4 %[[TMP]])
+// OGCG: call void @_Z17takeCopyCtorByref12WithCopyCtor(ptr nofree noundef align 4 dead_on_return dereferenceable(4) %[[TMP]])
+
+// 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..c3cf6c32c2005 100644
--- a/clang/test/CIR/Transforms/abi-lowering/indirect-byval.cir
+++ b/clang/test/CIR/Transforms/abi-lowering/indirect-byval.cir
@@ -38,6 +38,30 @@
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 } ]
+}
+
+#byref_then_direct = {
+ return = { kind = "direct" },
+ args = [ { kind = "indirect", indirect_align = 8, byval = false },
+ { kind = "direct" } ]
+}
+
#passthrough = {
return = { kind = "direct" },
args = [ ]
@@ -278,19 +302,153 @@ 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.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: %{{.*}} = 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: %{{.*}} = 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: 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: cir.call @takes_two_byref(%[[TMP]], %[[TMP]]) :
+ // CHECK-SAME: llvm.byref = !rec_Big
+
+ // The Direct operand keeps reading the load, so the load survives the rewrite.
+ cir.func private @takes_byref_then_direct(%a: !rec_Big, %b: !rec_Big)
+ attributes { test_classify = #byref_then_direct }
+
+ cir.func @caller_byref_then_direct(%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_byref_then_direct(%v, %v) : (!rec_Big, !rec_Big) -> ()
+ cir.return
+ }
+
+ // CHECK: cir.func{{.*}} @caller_byref_then_direct
+ // CHECK: %[[TMP:.*]] = cir.alloca "agg.tmp" align(8) : !cir.ptr<!rec_Big>
+ // CHECK: %[[V:.*]] = cir.load %[[TMP]] : !cir.ptr<!rec_Big>, !rec_Big
+ // CHECK: cir.call @takes_byref_then_direct(%[[TMP]], %[[V]]) :
+ // 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.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
More information about the cfe-commits
mailing list