https://github.com/skc7 updated https://github.com/llvm/llvm-project/pull/230415
>From 7df48138885295b78e5f701f8a97da8855e5bf37 Mon Sep 17 00:00:00 2001 From: skc7 <[email protected]> Date: Fri, 9 Oct 2026 14:13:57 +0530 Subject: [PATCH 1/2] [CIR][mlir][ABI] Add IndirectAliased (byref) argument lowering --- .../Transforms/CallConvLoweringPass.cpp | 11 +- .../TargetLowering/CIRABIRewriteContext.cpp | 173 +++++++++++----- .../byval-sret-arg-attr-lowering.cir | 15 ++ .../abi-lowering/indirect-aliased-nyi.cir | 64 ++++++ .../abi-lowering/indirect-aliased.cir | 196 ++++++++++++++++++ mlir/include/mlir/ABI/ABIRewriteContext.h | 19 +- .../mlir/ABI/Targets/Test/TestTarget.h | 5 +- mlir/lib/ABI/Targets/Test/TestTarget.cpp | 22 +- mlir/unittests/ABI/ABIRewriteContextTest.cpp | 10 + mlir/unittests/ABI/TestTargetTest.cpp | 48 +++++ 10 files changed, 497 insertions(+), 66 deletions(-) create mode 100644 clang/test/CIR/Transforms/abi-lowering/indirect-aliased-nyi.cir create mode 100644 clang/test/CIR/Transforms/abi-lowering/indirect-aliased.cir diff --git a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp index 9f736b920fcc02..a91533c1f582d5 100644 --- a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp +++ b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp @@ -575,6 +575,9 @@ static const llvm::abi::Type *mapCIRType(mlir::Type type, /// wide to fit (a `_BitInt` over 128 bits), is passed via a pointer (sret /// for returns, byval for arguments). /// +/// IndirectAliased: an argument passed as a pointer to the caller's object +/// with no copy implied (byref), in the address space the classifier names. +/// /// Ignore: a void return, or an empty record dropped from the signature. static std::optional<ArgClassification> convertABIArgInfo(const llvm::abi::ArgInfo &info, MLIRContext *ctx, @@ -642,6 +645,9 @@ convertABIArgInfo(const llvm::abi::ArgInfo &info, MLIRContext *ctx, return ArgClassification::getIndirect(info.getIndirectAlign(), info.getIndirectByVal(), info.getIndirectAddrSpace()); + if (info.isIndirectAliased()) + return ArgClassification::getIndirectAliased(info.getIndirectAlign(), + info.getIndirectAddrSpace()); // AArch64 pure scalable aggregates use CoerceAndExpand. This bridge lowers // x86_64 classifications only. assert(!info.isCoerceAndExpand() && @@ -808,12 +814,13 @@ CIRABIClassifierCommon::classifyVariadicCall( } /// Whether \p fc gives the callee access to memory through a pointer the ABI -/// introduced: an sret slot for an indirect return, or an indirect argument. +/// introduced: an sret slot for an indirect return, or an indirect or byref +/// argument. static bool hasIndirectSlot(const FunctionClassification &fc) { if (fc.returnInfo.kind == ArgKind::Indirect) return true; return llvm::any_of(fc.argInfos, [](const ArgClassification &ac) { - return ac.kind == ArgKind::Indirect; + return ac.kind == ArgKind::Indirect || ac.kind == ArgKind::IndirectAliased; }); } diff --git a/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp b/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp index c7ca6151029eb4..970452a60d07f8 100644 --- a/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp +++ b/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp @@ -28,7 +28,8 @@ using namespace mlir::abi; // This rewrite context supports the Direct (with or without coercion), // Extend, Ignore, Indirect-return (sret), Indirect-argument (byval and -// non-byval), and Expand (struct flattening) classifications. +// non-byval), IndirectAliased-argument (byref), and Expand (struct +// flattening) classifications. // // An Indirect argument is byval or not, following its classification's // byVal flag. byval is a by-value parameter the ABI passes in memory rather @@ -44,6 +45,10 @@ using namespace mlir::abi; // rewires that slot to the incoming pointer so the body mutates the caller's // storage in place. // +// A byref argument is lowered like byval, apart from its attribute and the +// address space of its pointer. The callee must not write through that +// pointer, so it fills the CIRGen param slot with a copy just as byval does. +// // 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 // directly into the parameter's own alloca (the CIRGen spill slot). At the @@ -60,6 +65,22 @@ using namespace mlir::abi; namespace { +/// A pointer to \p pointee in target address space \p addrSpace. Zero is the +/// default address space, which carries no attribute. +cir::PointerType getPtrTyInAddrSpace(mlir::Type pointee, unsigned addrSpace) { + mlir::ptr::MemorySpaceAttrInterface asAttr; + if (addrSpace) + asAttr = cir::TargetAddressSpaceAttr::get(pointee.getContext(), addrSpace); + return cir::PointerType::get(pointee, asAttr); +} + +/// The pointer passed in place of a byref value of type \p pointee. +cir::PointerType getByRefPtrTy(mlir::Type pointee, + const ArgClassification &ac) { + assert(ac.kind == ArgKind::IndirectAliased && "expected a byref argument"); + return getPtrTyInAddrSpace(pointee, ac.indirectAddrSpace); +} + /// Return the coerced RecordType for a Direct classification that should be /// flattened into individual scalar arguments, or a null type if the /// classification does not call for flattening. @@ -82,7 +103,8 @@ cir::RecordType getFlattenedCoercedType(const ArgClassification &ac) { /// Build the new argument-type list for a function whose ABI classification /// is \p fc. Handles Direct (with or without coercion), Extend, Ignore, -/// Indirect (byval and non-byval), and Expand (struct flattening) arguments. +/// Indirect (byval and non-byval), IndirectAliased (byref), and Expand +/// (struct flattening) arguments. /// The sret return pointer, when present, is prepended by /// rewriteFunctionDefinition rather than here. mlir::LogicalResult @@ -135,6 +157,9 @@ buildNewArgTypes(ArrayRef<mlir::Type> oldArgTypes, case ArgKind::Indirect: newArgTypes.push_back(cir::PointerType::get(origTy)); break; + case ArgKind::IndirectAliased: + newArgTypes.push_back(getByRefPtrTy(origTy, ac)); + break; } } return mlir::success(); @@ -142,8 +167,8 @@ buildNewArgTypes(ArrayRef<mlir::Type> oldArgTypes, /// Compute the new return type for a function whose return classification /// is \p retInfo. Direct returns keep (or coerce to) their type, Ignore and -/// Indirect (sret) returns become void, Extend keeps its type; Expand emits -/// an error. +/// Indirect (sret) returns become void, Extend keeps its type; Expand and +/// IndirectAliased emit an error. mlir::Type computeNewReturnType(mlir::Type origRetTy, const ArgClassification &retInfo, mlir::MLIRContext *ctx, @@ -159,6 +184,10 @@ computeNewReturnType(mlir::Type origRetTy, const ArgClassification &retInfo, emitError() << "Expand return is not allowed (classic codegen rejects " << "it in EmitFunctionEpilog)"; return nullptr; + case ArgKind::IndirectAliased: + emitError() << "IndirectAliased return is not allowed (classic codegen " + << "rejects it in EmitFunctionEpilog)"; + return nullptr; case ArgKind::Extend: // Same convention as Extend args: keep the original return type in the // signature; the sign/zero extension is communicated via the @@ -199,25 +228,27 @@ static bool isCoercionWiderThanMemory(mlir::Type origTy, mlir::Type coercedTy, } /// Whether updateArgAttrs has anything to change: an arg that is Ignore (its -/// slot is dropped), Extend (needs llvm.signext / llvm.zeroext), Indirect -/// (needs the pointer attributes), Expand (changes the argument count), or a -/// coerced Direct, which may be flattened or may lose llvm.noundef. +/// slot is dropped), Extend (needs llvm.signext / llvm.zeroext), Indirect or +/// IndirectAliased (needs the pointer attributes), Expand (changes the argument +/// count), or a coerced Direct, which may be flattened or may lose +/// llvm.noundef. static bool needsArgAttrUpdate(const FunctionClassification &fc) { return llvm::any_of(fc.argInfos, [](const ArgClassification &ac) { return ac.kind == ArgKind::Ignore || ac.kind == ArgKind::Extend || - ac.kind == ArgKind::Indirect || ac.kind == ArgKind::Expand || + ac.kind == ArgKind::Indirect || + ac.kind == ArgKind::IndirectAliased || ac.kind == ArgKind::Expand || (ac.kind == ArgKind::Direct && ac.coercedType); }); } /// Build an updated arg_attrs ArrayAttr that drops Ignore'd args, adds /// llvm.signext / llvm.zeroext on Extend args, adds the pointer attributes -/// for Indirect args in place of llvm.nofpclass, drops llvm.noundef from a -/// coerced Direct arg that passes undefined bits, gives each field of a -/// Direct+canFlatten argument the argument's attributes, and gives each field -/// of an Expand argument an empty dict. Direct and Extend slots keep their -/// other existing attributes. \p origArgTypes provides the pre-rewrite type -/// for each arg slot. +/// for Indirect args in place of llvm.nofpclass and for IndirectAliased args, +/// drops llvm.noundef from a coerced Direct arg that passes undefined bits, +/// gives each field of a Direct+canFlatten argument the argument's attributes, +/// and gives each field of an Expand argument an empty dict. Direct and +/// Extend slots keep their other existing attributes. \p origArgTypes +/// provides the pre-rewrite type for each arg slot. mlir::ArrayAttr updateArgAttrs(mlir::MLIRContext *ctx, ArrayRef<mlir::Type> origArgTypes, mlir::ArrayAttr existingArgAttrs, @@ -290,6 +321,16 @@ mlir::ArrayAttr updateArgAttrs(mlir::MLIRContext *ctx, dl.getTypeSize(pointeeTy).getFixedValue())); } newArgAttrs.push_back(attrs.getDictionary(ctx)); + } else if (ac.kind == ArgKind::IndirectAliased) { + // llvm.byref(T) records the pre-rewrite arg type, as llvm.byval does. + mlir::NamedAttrList attrs(existing); + attrs.set(mlir::LLVM::LLVMDialect::getAlignAttrName(), + builder.getI64IntegerAttr(ac.indirectAlign.value())); + attrs.set(mlir::LLVM::LLVMDialect::getNoUndefAttrName(), + builder.getUnitAttr()); + attrs.set(mlir::LLVM::LLVMDialect::getByRefAttrName(), + mlir::TypeAttr::get(origArgTypes[oldIdx])); + newArgAttrs.push_back(attrs.getDictionary(ctx)); } else { newArgAttrs.push_back(existing); } @@ -862,28 +903,28 @@ static cir::LoadOp getWholeRecordLoad(mlir::Value recordVal) { /// Returns \p load if the address it read from can be passed in place of the /// loaded value for an indirect argument that needs \p minAlign alignment, -/// and null otherwise, including when \p load is null. A byval argument -/// also needs storageUnwrittenBetween. On null the caller reports a -/// non-byval argument or fills a byval slot of its own. +/// and null otherwise, including when \p load is null. A byval or byref +/// argument also needs storageUnwrittenBetween. On null the caller reports a +/// non-byval argument or fills a slot of its own. /// -/// The address has to be a pointer to the loaded type in the default address -/// space, since that is what the rewritten parameter is, and a cir.load pins -/// the pointee type without pinning the address space. The storage it names -/// has to be a slot allocated here, reached through storage-preserving casts, -/// already stating at least \p minAlign. The enclosing function's own -/// non-byval parameter qualifies as well, since its slot stands until -/// finalizeParameterSlots and prepareNonByvalParameters has restated it with -/// the alignment that parameter promises rather than the one CIRGen chose -/// for a local copy. A byval parameter's slot keeps CIRGen's alignment, so -/// it qualifies only where that already covers \p minAlign. +/// The address has to have \p paramTy, the rewritten parameter's type, since a +/// cir.load pins the pointee type without pinning the address space. The +/// storage it names has to be a slot allocated here, reached through +/// storage-preserving casts, already stating at least \p minAlign. The +/// enclosing function's own non-byval parameter qualifies as well, since its +/// slot stands until finalizeParameterSlots and prepareNonByvalParameters has +/// restated it with the alignment that parameter promises rather than the one +/// CIRGen chose for a local copy. A byval or byref parameter's slot keeps +/// CIRGen's alignment, so it qualifies only where that already covers +/// \p minAlign. /// /// Raising a slot's alignment here would not hold for one that stands in /// for a parameter, since finalizeParameterSlots replaces it with the /// incoming pointer, which promises only what the caller gave it. static cir::LoadOp forwardableIndirectLoad(cir::LoadOp load, + cir::PointerType paramTy, uint64_t minAlign) { - if (!load || load.getAddr().getType() != - cir::PointerType::get(load.getResult().getType())) + if (!load || load.getAddr().getType() != paramTy) return {}; cir::AllocaOp slot = cir::getUnderlyingAlloca(load.getAddr()); return slot && slot.getAlignment() >= minAlign ? load : cir::LoadOp(); @@ -1004,17 +1045,18 @@ findParamSpill(mlir::BlockArgument blockArg) { /// 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, which find a record in the coercion slot -/// through rewireRecordUses. For each Indirect byval arg, change the block -/// argument's type to a pointer, then replace a record's single param spill -/// with a copy from that pointer. Without such a spill, a record's uses find -/// it at the pointer through rewireRecordUses, and a value of any other type is -/// loaded from the pointer at entry. For each Indirect non-byval arg, change -/// the block argument to a pointer and queue the param-slot alloca to be -/// replaced by it (no entry load or 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. +/// through rewireRecordUses. For each Indirect byval arg and each +/// IndirectAliased arg, change the block argument's type to a pointer, then +/// replace a record's single param spill with a copy from that pointer. +/// Without such a spill, a record's uses find it at the pointer through +/// rewireRecordUses, and a value of any other type is loaded from the pointer +/// at entry. For each Indirect non-byval arg, change the block argument to a +/// pointer and queue the param-slot alloca to be replaced by it (no entry load +/// or 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. /// /// \p hasSRetArg is true when the function has an sret return (a hidden return /// pointer is prepended as block argument 0). Expand arguments expand the @@ -1195,16 +1237,22 @@ insertArgCoercion(mlir::FunctionOpInterface funcOp, blockArg, blockArg, &entry, dl, coercionOps, ac.directOffset))) return mlir::failure(); - } else if (ac.kind == ArgKind::Indirect) { + } else if (ac.kind == ArgKind::Indirect || + ac.kind == ArgKind::IndirectAliased) { // byval and non-byval both lower to !cir.ptr<T>, and which it is shows // up only in the attrs updateArgAttrs applies. Body lowering differs: // the body addresses a byval parameter through the CIRGen slot it was // spilled into, so that slot has to be filled from the incoming // pointer, while a non-byval parameter's slot is replaced by the - // pointer so the body works on the caller's storage in place. - auto ptrTy = cir::PointerType::get(blockArg.getType()); - - if (!ac.byVal) { + // pointer so the body works on the caller's storage in place. A byref + // parameter takes the byval path, through a pointer in its own address + // space. + bool byRef = ac.kind == ArgKind::IndirectAliased; + cir::PointerType ptrTy = byRef + ? getByRefPtrTy(blockArg.getType(), ac) + : cir::PointerType::get(blockArg.getType()); + + if (!byRef && !ac.byVal) { // Without byval, drop the spill store and let the slot's uses read the // incoming pointer, so the body operates on the caller's storage in // place. A byte-copy would be wrong for non-trivially-copyable @@ -2070,8 +2118,15 @@ CIRABIRewriteContext::rewriteCallSite(mlir::Operation *callOp, newArgs.push_back(emitCoercion(builder, call.getLoc(), ac.coercedType, arg, slotBlock, dl, ac.directOffset)); } - } else if (ac.kind == ArgKind::Indirect) { + } else if (ac.kind == ArgKind::Indirect || + ac.kind == ArgKind::IndirectAliased) { uint64_t minAlign = ac.indirectAlign.value(); + // The callee copies a byref object at entry, so the caller treats it + // like byval. + bool byRef = ac.kind == ArgKind::IndirectAliased; + bool byCopy = byRef || ac.byVal; + cir::PointerType ptrTy = byRef ? getByRefPtrTy(arg.getType(), ac) + : cir::PointerType::get(arg.getType()); // A record operand that names storage is passed from that storage // rather than from the loaded value: a record value carries only the @@ -2080,7 +2135,7 @@ CIRABIRewriteContext::rewriteCallSite(mlir::Operation *callOp, // A byval _BitInt is passed from its value, since the load and the // store below are its conversion between value and in-memory form. cir::LoadOp srcLoad; - if (!ac.byVal || mlir::isa<cir::RecordType>(arg.getType())) + if (!byCopy || mlir::isa<cir::RecordType>(arg.getType())) srcLoad = maybeGetSimpleLoad(arg); // A non-byval argument must name the caller's storage, so that the @@ -2088,8 +2143,8 @@ CIRABIRewriteContext::rewriteCallSite(mlir::Operation *callOp, // means passing the address the operand was loaded from rather than the // loaded value, so a store to that storage after the load is visible to // the callee. byval fills a slot below, this cannot. - if (!ac.byVal) { - cir::LoadOp fwdLoad = forwardableIndirectLoad(srcLoad, minAlign); + if (!byCopy) { + cir::LoadOp fwdLoad = forwardableIndirectLoad(srcLoad, ptrTy, minAlign); if (!fwdLoad) return call->emitOpError() << "non-byval indirect argument that does not name the " @@ -2106,7 +2161,8 @@ CIRABIRewriteContext::rewriteCallSite(mlir::Operation *callOp, // call, since the operand carries the value as of the load. A volatile // load is not forwarded, since the copy byval takes at the call is not // volatile. - if (cir::LoadOp fwdLoad = forwardableIndirectLoad(srcLoad, minAlign); + if (cir::LoadOp fwdLoad = + forwardableIndirectLoad(srcLoad, ptrTy, minAlign); fwdLoad && storageUnwrittenBetween(fwdLoad, call.getOperation(), aliasAnalysis)) { newArgs.push_back(fwdLoad.getAddr()); @@ -2114,21 +2170,28 @@ CIRABIRewriteContext::rewriteCallSite(mlir::Operation *callOp, continue; } + // A byref slot has to be an alloca of the parameter's own type, which + // only the alloca address space can hold. + if (byRef && ptrTy != getAllocaPtrTy(arg.getType(), dl)) + return call->emitOpError() + << "byref argument outside the alloca address space that does " + "not name forwardable storage is not yet implemented in " + "CallConvLowering"; + // Otherwise fill a slot, at the load's position when there is one so // the copy reads the memory state the load read. if (failed(rejectAtomicRecordLoad(call, arg))) return mlir::failure(); if (!srcLoad) srcLoad = maybeGetCopyableRecordLoad(arg); - auto ptrTy = cir::PointerType::get(arg.getType()); mlir::IntegerAttr slotAlign = builder.getI64IntegerAttr(minAlign); mlir::OpBuilder::InsertionGuard guard(builder); if (srcLoad) builder.setInsertionPoint(srcLoad); - auto slot = - cir::AllocaOp::create(builder, call.getLoc(), ptrTy, - builder.getStringAttr("byval"), slotAlign); + auto slot = cir::AllocaOp::create( + builder, call.getLoc(), ptrTy, + builder.getStringAttr(byRef ? "byref" : "byval"), slotAlign); if (srcLoad) { auto copy = cir::CopyOp::create(builder, call.getLoc(), slot, srcLoad.getAddr(), slotAlign, @@ -2548,6 +2611,8 @@ CIRABIRewriteContext::rewriteVAArg(mlir::Operation *vaArgOp, if (ac.kind == ArgKind::Indirect && !ac.byVal) return reportVAArgNYI(op, "a non-trivially-copyable type"); + if (ac.kind == ArgKind::IndirectAliased) + return reportVAArgNYI(op, "a type passed byref"); // neededInt counts 8-byte integer slots and neededSse counts 16-byte vector // slots. Zero of both means the type travels in memory and is read diff --git a/clang/test/CIR/Transforms/abi-lowering/byval-sret-arg-attr-lowering.cir b/clang/test/CIR/Transforms/abi-lowering/byval-sret-arg-attr-lowering.cir index b0df1caa5416b4..ffc6ea00ceda7c 100644 --- a/clang/test/CIR/Transforms/abi-lowering/byval-sret-arg-attr-lowering.cir +++ b/clang/test/CIR/Transforms/abi-lowering/byval-sret-arg-attr-lowering.cir @@ -23,6 +23,12 @@ args = [ { kind = "indirect", indirect_align = 8, byval = false } ] } +#byref_as4_arg = { + return = { kind = "direct" }, + args = [ { kind = "indirect_aliased", indirect_align = 8, + indirect_addr_space = 4 } ] +} + #passthrough = { return = { kind = "direct" }, args = [ ] @@ -64,6 +70,15 @@ module attributes { // CHECK: cir.func{{.*}} @takes_non_byval(%{{.*}}: !cir.ptr<!rec_Big> {{{.*}}llvm.dereferenceable = 24 : i64, llvm.nofreeobj, llvm.noundef{{.*}}}) // LLVM: define void @takes_non_byval(ptr nofreeobj noundef align 8 dereferenceable(24) %{{.+}}) + // The byref argument attribute is converted like byval, on a pointer in its + // own address space. + cir.func @takes_big_byref(%arg0: !rec_Big) attributes { test_classify = #byref_as4_arg } { + cir.return + } + + // CHECK: cir.func{{.*}} @takes_big_byref(%{{.*}}: !cir.ptr<!rec_Big, target_address_space(4)> {{{.*}}llvm.byref = !rec_Big{{.*}}}) + // LLVM: define void @takes_big_byref(ptr addrspace(4) noundef byref(%struct.Big) align 8 %{{.+}}) + // The call site's byval operand attribute is converted too. cir.func @caller(%s: !rec_Big) attributes { test_classify = #passthrough } { cir.call @takes_big(%s) : (!rec_Big) -> () diff --git a/clang/test/CIR/Transforms/abi-lowering/indirect-aliased-nyi.cir b/clang/test/CIR/Transforms/abi-lowering/indirect-aliased-nyi.cir new file mode 100644 index 00000000000000..adcb86f35eb51e --- /dev/null +++ b/clang/test/CIR/Transforms/abi-lowering/indirect-aliased-nyi.cir @@ -0,0 +1,64 @@ +// 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_return = { + return = { kind = "indirect_aliased", indirect_align = 8 }, + args = [ ] +} + +module attributes { + dlti.dl_spec = #dlti.dl_spec< + #dlti.dl_entry<i64, dense<64>: vector<2xi64>>> +} { + + cir.func private @ret_big() -> !rec_Big + attributes { test_classify = #byref_return } + + // CHECK: error: 'cir.func' op IndirectAliased return is not allowed + // CHECK-SAME: (classic codegen rejects it in EmitFunctionEpilog) + +} + +// ----- + +!s64i = !cir.int<s, 64> +!rec_Big = !cir.struct<"Big" {data !s64i, data !s64i, data !s64i, data !s64i}> + +#byref_as4 = { + return = { kind = "direct" }, + args = [ { kind = "indirect_aliased", indirect_align = 8, + indirect_addr_space = 4 } ] +} + +#passthrough = { + return = { kind = "direct" }, + args = [ ] +} + +module attributes { + dlti.dl_spec = #dlti.dl_spec< + #dlti.dl_entry<i64, dense<64>: vector<2xi64>>, + #dlti.dl_entry<"dlti.alloca_memory_space", 5 : ui64>> +} { + + cir.func private @takes_big_as4(%arg0: !rec_Big) + attributes { test_classify = #byref_as4 } + + // A slot for the copy would be an alloca, which cannot live in address + // space 4. + cir.func @caller_as4_no_storage(%novalue: !rec_Big) + attributes { test_classify = #passthrough } { + cir.call @takes_big_as4(%novalue) : (!rec_Big) -> () + cir.return + } + + // CHECK: error: 'cir.call' op byref argument outside the alloca address + // CHECK-SAME: space that does not name forwardable storage is not yet + // CHECK-SAME: implemented in CallConvLowering + // CHECK-NEXT: cir.call @takes_big_as4(%novalue) + +} diff --git a/clang/test/CIR/Transforms/abi-lowering/indirect-aliased.cir b/clang/test/CIR/Transforms/abi-lowering/indirect-aliased.cir new file mode 100644 index 00000000000000..f4e5ef30ee502f --- /dev/null +++ b/clang/test/CIR/Transforms/abi-lowering/indirect-aliased.cir @@ -0,0 +1,196 @@ +// RUN: cir-opt %s -split-input-file \ +// RUN: -cir-call-conv-lowering="classification-attr=test_classify" \ +// RUN: | FileCheck %s + +// IndirectAliased (byref) arguments with allocas in address space 5, as on +// AMDGPU. A kernel takes its byref arguments in address space 4. + +!s32i = !cir.int<s, 32> +!s64i = !cir.int<s, 64> +!rec_Big = !cir.struct<"Big" {data !s64i, data !s64i, data !s64i, data !s64i}> + +#byref_as4 = { + return = { kind = "direct" }, + args = [ { kind = "indirect_aliased", indirect_align = 8, + indirect_addr_space = 4 } ] +} + +#byref_as5 = { + return = { kind = "direct" }, + args = [ { kind = "indirect_aliased", indirect_align = 8, + indirect_addr_space = 5 } ] +} + +#passthrough = { + return = { kind = "direct" }, + args = [ ] +} + +module attributes { + dlti.dl_spec = #dlti.dl_spec< + #dlti.dl_entry<i64, dense<64>: vector<2xi64>>, + #dlti.dl_entry<"dlti.alloca_memory_space", 5 : ui64>> +} { + + // The callee must not write through a byref pointer, so the CIRGen slot is + // filled by a copy across address spaces. + cir.func @kernel_field(%arg0: !rec_Big) -> !s64i + attributes { test_classify = #byref_as4 } { + %0 = cir.alloca "arg0" align(8) init + : !cir.ptr<!rec_Big, target_address_space(5)> + cir.store %arg0, %0 : !rec_Big, !cir.ptr<!rec_Big, target_address_space(5)> + %1 = cir.get_member %0[0] {name = "a"} + : !cir.ptr<!rec_Big, target_address_space(5)> + -> !cir.ptr<!s64i, target_address_space(5)> + %2 = cir.load %1 : !cir.ptr<!s64i, target_address_space(5)>, !s64i + cir.return %2 : !s64i + } + + // CHECK: cir.func{{.*}} @kernel_field(%[[PTR:.*]]: !cir.ptr<!rec_Big, target_address_space(4)> + // CHECK-SAME: {llvm.align = 8 : i64, llvm.byref = !rec_Big, llvm.noundef}) + // CHECK: %[[SLOT:.*]] = cir.alloca "arg0" align(8) init : !cir.ptr<!rec_Big, target_address_space(5)> + // CHECK-NOT: cir.store + // CHECK: cir.copy %[[PTR]] align(8) to %[[SLOT]] align(8) + // CHECK-SAME: : !cir.ptr<!rec_Big, target_address_space(4)>, !cir.ptr<!rec_Big, target_address_space(5)> + // CHECK: cir.get_member %[[SLOT]][0] + + // A parameter the body never reads gets nothing at entry. + cir.func @kernel_unused(%arg0: !rec_Big) -> !s32i + attributes { test_classify = #byref_as4 } { + %r = cir.const #cir.int<0> : !s32i + cir.return %r : !s32i + } + + // CHECK: cir.func{{.*}} @kernel_unused(%{{.*}}: !cir.ptr<!rec_Big, target_address_space(4)> + // CHECK-SAME: {llvm.align = 8 : i64, llvm.byref = !rec_Big, llvm.noundef}) + // CHECK-NEXT: %{{.*}} = cir.const #cir.int<0> : !s32i + + // A body that reads the value with no spill loads it at entry. + cir.func @kernel_uses_arg(%arg0: !rec_Big) -> !rec_Big + attributes { test_classify = #byref_as4 } { + cir.return %arg0 : !rec_Big + } + + // CHECK: cir.func{{.*}} @kernel_uses_arg(%[[PTR:.*]]: !cir.ptr<!rec_Big, target_address_space(4)> + // CHECK: %[[V:.*]] = cir.load %[[PTR]] : !cir.ptr<!rec_Big, target_address_space(4)>, !rec_Big + // CHECK-NEXT: cir.return %[[V]] : !rec_Big + + cir.func private @takes_big_as5(%arg0: !rec_Big) + attributes { test_classify = #byref_as5 } + + // CHECK: cir.func{{.*}} @takes_big_as5(!cir.ptr<!rec_Big, target_address_space(5)> + // CHECK-SAME: {llvm.align = 8 : i64, llvm.byref = !rec_Big, llvm.noundef}) + + // Storage in the parameter's own address space is handed on. + cir.func @caller_forward(%s: !rec_Big) + attributes { test_classify = #passthrough } { + %tmp = cir.alloca "agg.tmp" align(8) + : !cir.ptr<!rec_Big, target_address_space(5)> + cir.store %s, %tmp : !rec_Big, !cir.ptr<!rec_Big, target_address_space(5)> + %v = cir.load %tmp : !cir.ptr<!rec_Big, target_address_space(5)>, !rec_Big + cir.call @takes_big_as5(%v) : (!rec_Big) -> () + cir.return + } + + // CHECK: cir.func{{.*}} @caller_forward + // CHECK: %[[TMP:.*]] = cir.alloca "agg.tmp" align(8) : !cir.ptr<!rec_Big, target_address_space(5)> + // CHECK-NOT: cir.alloca "byref" + // CHECK-NOT: cir.load + // CHECK: cir.call @takes_big_as5(%[[TMP]]) : (!cir.ptr<!rec_Big, target_address_space(5)> + // CHECK-SAME: {llvm.align = 8 : i64, llvm.byref = !rec_Big, llvm.noundef}) + + // An operand that names no storage is stored into a slot in the alloca + // address space. + cir.func @caller_no_storage(%s: !rec_Big) + attributes { test_classify = #passthrough } { + cir.call @takes_big_as5(%s) : (!rec_Big) -> () + cir.return + } + + // CHECK: cir.func{{.*}} @caller_no_storage(%[[S:.*]]: !rec_Big) + // CHECK: %[[SLOT:.*]] = cir.alloca "byref" align(8) : !cir.ptr<!rec_Big, target_address_space(5)> + // CHECK-NEXT: cir.store %[[S]], %[[SLOT]] : !rec_Big, !cir.ptr<!rec_Big, target_address_space(5)> + // CHECK-NEXT: cir.call @takes_big_as5(%[[SLOT]]) + + // A store between the load and the call stops the storage from being handed + // on, so the copy is taken where the load was. + cir.func @caller_store_between(%s: !rec_Big, %t: !rec_Big) + attributes { test_classify = #passthrough } { + %tmp = cir.alloca "agg.tmp" align(8) + : !cir.ptr<!rec_Big, target_address_space(5)> + cir.store %s, %tmp : !rec_Big, !cir.ptr<!rec_Big, target_address_space(5)> + %v = cir.load %tmp : !cir.ptr<!rec_Big, target_address_space(5)>, !rec_Big + cir.store %t, %tmp : !rec_Big, !cir.ptr<!rec_Big, target_address_space(5)> + cir.call @takes_big_as5(%v) : (!rec_Big) -> () + cir.return + } + + // CHECK: cir.func{{.*}} @caller_store_between(%[[S:[^:]+]]: !rec_Big, %[[T:[^:]+]]: !rec_Big) + // CHECK: %[[TMP:.*]] = cir.alloca "agg.tmp" align(8) : !cir.ptr<!rec_Big, target_address_space(5)> + // CHECK-NEXT: cir.store %[[S]], %[[TMP]] + // CHECK-NEXT: %[[SLOT:.*]] = cir.alloca "byref" align(8) : !cir.ptr<!rec_Big, target_address_space(5)> + // CHECK-NEXT: cir.copy %[[TMP]] align(8) to %[[SLOT]] align(8) + // CHECK-NEXT: cir.store %[[T]], %[[TMP]] + // CHECK-NEXT: cir.call @takes_big_as5(%[[SLOT]]) + +} + +// ----- + +// byref in the default address space with no alloca address space set. + +!s64i = !cir.int<s, 64> +!rec_Big = !cir.struct<"Big" {data !s64i, data !s64i, data !s64i, data !s64i}> + +#byref_as0 = { + return = { kind = "direct" }, + args = [ { kind = "indirect_aliased", indirect_align = 8 } ] +} + +#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(%arg0: !rec_Big) + attributes { test_classify = #byref_as0 } + + // CHECK: cir.func{{.*}} @takes_big(!cir.ptr<!rec_Big> + // CHECK-SAME: {llvm.align = 8 : i64, llvm.byref = !rec_Big, llvm.noundef}) + + cir.func @caller_forward(%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_big(%v) : (!rec_Big) -> () + cir.return + } + + // CHECK: cir.func{{.*}} @caller_forward + // CHECK: %[[TMP:.*]] = cir.alloca "agg.tmp" align(8) : !cir.ptr<!rec_Big> + // CHECK-NOT: cir.load + // CHECK: cir.call @takes_big(%[[TMP]]) : (!cir.ptr<!rec_Big> + + // Storage aligned below what the callee expects gets a slot of its own. + cir.func @caller_underaligned(%s: !rec_Big) + attributes { test_classify = #passthrough } { + %tmp = cir.alloca "agg.tmp" align(1) : !cir.ptr<!rec_Big> + cir.store %s, %tmp : !rec_Big, !cir.ptr<!rec_Big> + %v = cir.load align(1) %tmp : !cir.ptr<!rec_Big>, !rec_Big + cir.call @takes_big(%v) : (!rec_Big) -> () + cir.return + } + + // CHECK: cir.func{{.*}} @caller_underaligned + // CHECK: %[[TMP:.*]] = cir.alloca "agg.tmp" align(1) : !cir.ptr<!rec_Big> + // CHECK: %[[SLOT:.*]] = cir.alloca "byref" align(8) : !cir.ptr<!rec_Big> + // CHECK-NEXT: cir.copy %[[TMP]] align(1) to %[[SLOT]] align(8) : !cir.ptr<!rec_Big> + // CHECK-NEXT: cir.call @takes_big(%[[SLOT]]) + +} diff --git a/mlir/include/mlir/ABI/ABIRewriteContext.h b/mlir/include/mlir/ABI/ABIRewriteContext.h index fb94cb5559fbcf..812245602bae95 100644 --- a/mlir/include/mlir/ABI/ABIRewriteContext.h +++ b/mlir/include/mlir/ABI/ABIRewriteContext.h @@ -48,6 +48,10 @@ enum class ArgKind : uint8_t { /// Pass indirectly via a pointer (sret for returns, byval for args). Indirect, + /// Pass a pointer to the caller's object with no copy implied (byref). The + /// callee must not write through it. Arguments only. + IndirectAliased, + /// Ignore (void return, empty struct). Ignore, @@ -64,7 +68,7 @@ struct ArgClassification { /// use the original type. Type coercedType = nullptr; - /// For Indirect: alignment of the pointed-to object. + /// For Indirect and IndirectAliased: alignment of the pointed-to object. llvm::Align indirectAlign = llvm::Align(1); /// For Extend: whether to sign-extend (true) or zero-extend (false). @@ -77,8 +81,8 @@ struct ArgClassification { /// For Indirect: whether the callee gets ownership (byval). bool byVal = false; - /// For Indirect: target address space of the pointer passed in place of - /// the value. Zero is the default address space. + /// For Indirect and IndirectAliased: target address space of the pointer + /// passed in place of the value. Zero is the default address space. unsigned indirectAddrSpace = 0; /// For Direct with coercion: the byte offset within the original aggregate @@ -143,6 +147,15 @@ struct ArgClassification { return c; } + static ArgClassification getIndirectAliased(llvm::Align align, + unsigned addrSpace) { + ArgClassification c; + c.kind = ArgKind::IndirectAliased; + c.indirectAlign = align; + c.indirectAddrSpace = addrSpace; + return c; + } + static ArgClassification getExtend(Type coerced, bool signExt) { ArgClassification c; c.kind = ArgKind::Extend; diff --git a/mlir/include/mlir/ABI/Targets/Test/TestTarget.h b/mlir/include/mlir/ABI/Targets/Test/TestTarget.h index 2036837b7d91a6..f1e637fc1f1c6b 100644 --- a/mlir/include/mlir/ABI/Targets/Test/TestTarget.h +++ b/mlir/include/mlir/ABI/Targets/Test/TestTarget.h @@ -59,7 +59,7 @@ FunctionClassification classify(TypeRange argTypes, Type returnType, /// /// Per-arg/return dictionary keys: /// kind: StringAttr. One of "direct", "extend", "indirect", -/// "ignore", "expand". +/// "indirect_aliased", "ignore", "expand". /// /// For kind = "direct" (all optional): /// coerced_type: TypeAttr. ABI-coerced type, if different from the @@ -78,6 +78,9 @@ FunctionClassification classify(TypeRange argTypes, Type returnType, /// indirect_addr_space: IntegerAttr. Target address space of the /// pointer. Defaults to 0. /// +/// For kind = "indirect_aliased": the same keys as "indirect" except byval, +/// which it rejects. +/// /// For kind = "ignore" / "expand": no extra keys. /// /// Future schema additions tracked in projects/daily_log.md (Step 0c diff --git a/mlir/lib/ABI/Targets/Test/TestTarget.cpp b/mlir/lib/ABI/Targets/Test/TestTarget.cpp index eb831a95a63971..492671dabe6cd4 100644 --- a/mlir/lib/ABI/Targets/Test/TestTarget.cpp +++ b/mlir/lib/ABI/Targets/Test/TestTarget.cpp @@ -179,10 +179,11 @@ parseOne(DictionaryAttr argDict, function_ref<InFlightDiagnostic()> emitError) { return ArgClassification::getExtend(coerced.getValue(), signExt); } - if (kind == "indirect") { + if (kind == "indirect" || kind == "indirect_aliased") { auto align = argDict.getAs<IntegerAttr>("indirect_align"); if (!align) { - emitError() << "kind='indirect' requires 'indirect_align' IntegerAttr"; + emitError() << "kind='" << kind + << "' requires 'indirect_align' IntegerAttr"; return std::nullopt; } if (align.getInt() <= 0 || !llvm::isPowerOf2_64(align.getInt())) { @@ -190,9 +191,6 @@ parseOne(DictionaryAttr argDict, function_ref<InFlightDiagnostic()> emitError) { << align.getInt(); return std::nullopt; } - bool byVal = true; - if (auto bv = argDict.getAs<BoolAttr>("byval")) - byVal = bv.getValue(); unsigned addrSpace = 0; if (auto as = argDict.getAs<IntegerAttr>("indirect_addr_space")) { if (as.getInt() < 0) { @@ -202,6 +200,17 @@ parseOne(DictionaryAttr argDict, function_ref<InFlightDiagnostic()> emitError) { } addrSpace = as.getInt(); } + if (kind == "indirect_aliased") { + if (argDict.get("byval")) { + emitError() << "kind='indirect_aliased' does not take 'byval'"; + return std::nullopt; + } + return ArgClassification::getIndirectAliased(llvm::Align(align.getInt()), + addrSpace); + } + bool byVal = true; + if (auto bv = argDict.getAs<BoolAttr>("byval")) + byVal = bv.getValue(); return ArgClassification::getIndirect(llvm::Align(align.getInt()), byVal, addrSpace); } @@ -217,7 +226,8 @@ parseOne(DictionaryAttr argDict, function_ref<InFlightDiagnostic()> emitError) { } emitError() << "unknown kind='" << kind - << "'; expected one of direct, extend, indirect, ignore, expand"; + << "'; expected one of direct, extend, indirect, " + "indirect_aliased, ignore, expand"; return std::nullopt; } diff --git a/mlir/unittests/ABI/ABIRewriteContextTest.cpp b/mlir/unittests/ABI/ABIRewriteContextTest.cpp index 47875b3d84100a..82a885cbf7b4c2 100644 --- a/mlir/unittests/ABI/ABIRewriteContextTest.cpp +++ b/mlir/unittests/ABI/ABIRewriteContextTest.cpp @@ -77,6 +77,16 @@ TEST(ABIRewriteContextTest, ArgClassificationIndirectAddrSpace) { EXPECT_FALSE(c == ArgClassification::getIndirect(llvm::Align(8), true)); } +TEST(ABIRewriteContextTest, ArgClassificationIndirectAliased) { + auto c = ArgClassification::getIndirectAliased(llvm::Align(8), 4); + EXPECT_EQ(c.kind, ArgKind::IndirectAliased); + EXPECT_EQ(c.indirectAlign, llvm::Align(8)); + EXPECT_EQ(c.indirectAddrSpace, 4u); + EXPECT_FALSE(c.byVal); + EXPECT_FALSE(c.isPassThrough()); + EXPECT_FALSE(c == ArgClassification::getIndirect(llvm::Align(8), false, 4)); +} + TEST(ABIRewriteContextTest, ArgClassificationExtend) { MLIRContext mlirCtx; auto i8 = IntegerType::get(&mlirCtx, 8); diff --git a/mlir/unittests/ABI/TestTargetTest.cpp b/mlir/unittests/ABI/TestTargetTest.cpp index c1438dd834a3c3..5fd77f09455084 100644 --- a/mlir/unittests/ABI/TestTargetTest.cpp +++ b/mlir/unittests/ABI/TestTargetTest.cpp @@ -220,6 +220,54 @@ TEST_F(TestTargetParseTest, ParsesIndirectAddrSpace) { EXPECT_EQ(fc.returnInfo.indirectAddrSpace, 5u); } +TEST_F(TestTargetParseTest, ParsesIndirectAliased) { + auto direct = + makeArg({builder.getNamedAttr("kind", builder.getStringAttr("direct"))}); + auto aliased = makeArg({ + builder.getNamedAttr("kind", builder.getStringAttr("indirect_aliased")), + builder.getNamedAttr("indirect_align", builder.getI64IntegerAttr(8)), + builder.getNamedAttr("indirect_addr_space", builder.getI64IntegerAttr(4)), + }); + auto attr = builder.getDictionaryAttr({ + builder.getNamedAttr("return", direct), + builder.getNamedAttr("args", builder.getArrayAttr({aliased})), + }); + + auto fc = parseOk(attr); + ASSERT_EQ(fc.argInfos.size(), 1u); + EXPECT_EQ(fc.argInfos[0].kind, ArgKind::IndirectAliased); + EXPECT_EQ(fc.argInfos[0].indirectAlign, llvm::Align(8)); + EXPECT_EQ(fc.argInfos[0].indirectAddrSpace, 4u); + EXPECT_FALSE(fc.argInfos[0].byVal); +} + +TEST_F(TestTargetParseTest, RejectsIndirectAliasedWithoutAlign) { + auto badAliased = makeArg({builder.getNamedAttr( + "kind", builder.getStringAttr("indirect_aliased"))}); + auto direct = + makeArg({builder.getNamedAttr("kind", builder.getStringAttr("direct"))}); + auto attr = builder.getDictionaryAttr({ + builder.getNamedAttr("return", direct), + builder.getNamedAttr("args", builder.getArrayAttr({badAliased})), + }); + parseError(attr, "kind='indirect_aliased' requires 'indirect_align'"); +} + +TEST_F(TestTargetParseTest, RejectsIndirectAliasedWithByval) { + auto badAliased = makeArg({ + builder.getNamedAttr("kind", builder.getStringAttr("indirect_aliased")), + builder.getNamedAttr("indirect_align", builder.getI64IntegerAttr(8)), + builder.getNamedAttr("byval", builder.getBoolAttr(true)), + }); + auto direct = + makeArg({builder.getNamedAttr("kind", builder.getStringAttr("direct"))}); + auto attr = builder.getDictionaryAttr({ + builder.getNamedAttr("return", direct), + builder.getNamedAttr("args", builder.getArrayAttr({badAliased})), + }); + parseError(attr, "kind='indirect_aliased' does not take 'byval'"); +} + TEST_F(TestTargetParseTest, ParsesIgnoreAndExpand) { auto ignore = makeArg({builder.getNamedAttr("kind", builder.getStringAttr("ignore"))}); >From f3ff3845e9db4714fa6ba2bc7c928a46aac1c811 Mon Sep 17 00:00:00 2001 From: skc7 <[email protected]> Date: Wed, 21 Oct 2026 14:39:57 +0530 Subject: [PATCH 2/2] update CI failing test --- clang/test/CIR/Transforms/abi-lowering/indirect-aliased.cir | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/CIR/Transforms/abi-lowering/indirect-aliased.cir b/clang/test/CIR/Transforms/abi-lowering/indirect-aliased.cir index f4e5ef30ee502f..5d5919bb808859 100644 --- a/clang/test/CIR/Transforms/abi-lowering/indirect-aliased.cir +++ b/clang/test/CIR/Transforms/abi-lowering/indirect-aliased.cir @@ -72,7 +72,7 @@ module attributes { } // CHECK: cir.func{{.*}} @kernel_uses_arg(%[[PTR:.*]]: !cir.ptr<!rec_Big, target_address_space(4)> - // CHECK: %[[V:.*]] = cir.load %[[PTR]] : !cir.ptr<!rec_Big, target_address_space(4)>, !rec_Big + // CHECK: %[[V:.*]] = cir.load align(8) %[[PTR]] : !cir.ptr<!rec_Big, target_address_space(4)>, !rec_Big // CHECK-NEXT: cir.return %[[V]] : !rec_Big cir.func private @takes_big_as5(%arg0: !rec_Big) _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
