https://github.com/skc7 created https://github.com/llvm/llvm-project/pull/230415
Summary: This PR includes the following changes: ABIRewriteContext: - Added `ArgKind::IndirectAliased` and `ArgClassification::getIndirectAliased` to the MLIR ABI library. CallConvLowering: - Mapped classic `IndirectAliased` to the new kind instead of asserting. CIRABIRewriteContext: - Rewrite a `byref` parameter to a pointer in its own address space. - Attach `llvm.align`, `llvm.noundef` and `llvm.byref(T)` to the parameter and to call operands. - Callee copies the object into its param slot, as for byval. - Call site forwards matching storage or copies into a "byref" slot. - Reject `byref` returns and `va_arg`. Assisted by : claude opus 5.5 >From 28ca81bdc240aefb88327ed51f3f862fafe1cba7 Mon Sep 17 00:00:00 2001 From: skc7 <[email protected]> Date: Fri, 9 Oct 2026 14:13:57 +0530 Subject: [PATCH] [CIR][mlir][ABI] Add IndirectAliased (byref) argument lowering --- .../Transforms/CallConvLoweringPass.cpp | 11 +- .../TargetLowering/CIRABIRewriteContext.cpp | 175 +++++++++++----- .../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, 499 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 1882b0b7a93e72..2a230df5d84a03 100644 --- a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp +++ b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp @@ -562,6 +562,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, @@ -628,6 +631,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()); assert(info.isIgnore() && "Unexpected classification"); return ArgClassification::getIgnore(); } @@ -798,12 +804,13 @@ static std::optional<FunctionClassification> classifyX86_64VariadicCall( } /// 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 1f69ae89d47665..83f84ed81b7f53 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 @@ -136,6 +158,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(); @@ -143,8 +168,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, @@ -160,6 +185,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 @@ -187,9 +216,9 @@ mlir::Value createIgnoredValue(mlir::OpBuilder &builder, mlir::Location loc, /// Build an updated arg_attrs ArrayAttr that drops Ignore'd args, adds /// llvm.signext / llvm.zeroext on Extend args, and adds the pointer -/// attributes for Indirect args. Preserves any existing arg attributes on -/// retained arg slots. \p origArgTypes provides the pre-rewrite type for -/// each arg slot. +/// attributes for Indirect and IndirectAliased args. Preserves any existing +/// arg attributes on retained arg slots. \p origArgTypes provides the +/// pre-rewrite type for each arg slot. mlir::ArrayAttr updateArgAttrs(mlir::MLIRContext *ctx, ArrayRef<mlir::Type> origArgTypes, mlir::ArrayAttr existingArgAttrs, @@ -255,6 +284,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); } @@ -623,28 +662,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(); @@ -764,16 +803,17 @@ 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. For each Indirect byval arg, -/// change the block argument's type to a pointer, then replace a record's -/// param spill with a copy from that pointer, or insert a load at entry for -/// a body that reads the parameter as a value. 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. +/// back to the original type for body uses. For each Indirect byval arg and +/// each IndirectAliased arg, change the block argument's type to a pointer, +/// then replace a record's param spill with a copy from that pointer, or +/// insert a load at entry for a body that reads the parameter as a value. +/// 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 @@ -957,16 +997,22 @@ void insertArgCoercion( // operand is blockArg, and if we naively replaceAllUses it gets swapped // to adapted (now of the original type != the alloca's pointee type). blockArg.replaceAllUsesExcept(adapted, coercionOps); - } 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 @@ -1283,8 +1329,9 @@ void rewriteIndirectReturnCall(cir::CallOp call, bool needsArgAttrUpdate = 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 || - getFlattenedCoercedType(ac); + ac.kind == ArgKind::Indirect || + ac.kind == ArgKind::IndirectAliased || + ac.kind == ArgKind::Expand || getFlattenedCoercedType(ac); }); if (needsArgAttrUpdate) argAttrs = updateArgAttrs(ctx, origCallArgTypes, argAttrs, fc, dl); @@ -1631,8 +1678,9 @@ mlir::LogicalResult CIRABIRewriteContext::rewriteFunctionDefinition( bool needsArgAttrUpdate = hasSRet || 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 || - getFlattenedCoercedType(ac); + ac.kind == ArgKind::Indirect || + ac.kind == ArgKind::IndirectAliased || + ac.kind == ArgKind::Expand || getFlattenedCoercedType(ac); }); if (needsArgAttrUpdate) { auto existing = funcOp->getAttrOfType<mlir::ArrayAttr>("arg_attrs"); @@ -1780,8 +1828,15 @@ CIRABIRewriteContext::rewriteCallSite(mlir::Operation *callOp, arg = emitCoercion(builder, call.getLoc(), ac.coercedType, arg, slotBlock, dl, ac.directOffset); newArgs.push_back(arg); - } 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 @@ -1790,7 +1845,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 @@ -1798,8 +1853,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 " @@ -1815,7 +1870,8 @@ CIRABIRewriteContext::rewriteCallSite(mlir::Operation *callOp, // one. That holds only while nothing writes that storage between the // load and the call, since the operand carries the value as of the // load. - 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()); @@ -1823,17 +1879,24 @@ 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. - 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) { cir::CopyOp::create(builder, call.getLoc(), slot, srcLoad.getAddr(), slotAlign, loadSourceAlignment(srcLoad, builder)); @@ -1891,13 +1954,15 @@ CIRABIRewriteContext::rewriteCallSite(mlir::Operation *callOp, // Layer llvm.signext / llvm.zeroext onto the new call's arg_attrs and // res_attrs for Extend args/return. Ignore args require a rebuild because - // their slots are dropped; Indirect args need llvm.byval / llvm.align; - // Expand and Direct+canFlatten args change the argument count. + // their slots are dropped; Indirect args need llvm.byval / llvm.align and + // byref args llvm.byref; Expand and Direct+canFlatten args change the + // argument count. bool needsArgAttrUpdate = 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 || - getFlattenedCoercedType(ac); + ac.kind == ArgKind::Indirect || + ac.kind == ArgKind::IndirectAliased || + ac.kind == ArgKind::Expand || getFlattenedCoercedType(ac); }); if (needsArgAttrUpdate) { auto existing = call->getAttrOfType<mlir::ArrayAttr>("arg_attrs"); @@ -2243,6 +2308,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"))}); _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
