https://github.com/adams381 updated https://github.com/llvm/llvm-project/pull/220724
>From 1644b94b8bb92f43cda31c08323c58c7b7fd79e1 Mon Sep 17 00:00:00 2001 From: Adam Smith <[email protected]> Date: Wed, 2 Sep 2026 13:40:05 -0700 Subject: [PATCH 1/5] [CIR] Honor the Direct coercion offset in x86_64 callconv lowering When a record's low eightbyte holds no field, the classifier passes it in one register read from byte 8 rather than byte 0. CallConvLowering had no way to represent that, so `convertABIArgInfo` rejected the whole signature as NYI. `ArgClassification` now carries the offset and `emitCoercionToMemory` reads and writes the coerced scalar through a `u8` `cir.ptr_stride` at that byte. The offset-zero path is untouched. A register-tuple coercion at an offset is refused rather than read from the wrong bytes, a pair SysV cannot currently produce. Supersedes #203640, which had the coercion mechanism but nothing calling it. Assisted-by: Cursor / claude-opus-5 --- .../Transforms/CallConvLoweringPass.cpp | 42 +++--- .../TargetLowering/CIRABIRewriteContext.cpp | 113 +++++++++++----- .../call-conv-lowering-x86_64-empty.cpp | 31 +++++ .../abi-lowering/x86_64-aggregate-nyi.cir | 10 -- .../x86_64-struct-direct-offset.cir | 123 ++++++++++++++++++ mlir/include/mlir/ABI/ABIRewriteContext.h | 17 ++- 6 files changed, 275 insertions(+), 61 deletions(-) create mode 100644 clang/test/CIR/Transforms/abi-lowering/x86_64-struct-direct-offset.cir diff --git a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp index 233b826853f76..242fda30343a1 100644 --- a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp +++ b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp @@ -473,14 +473,16 @@ static const llvm::abi::Type *mapCIRType(mlir::Type type, /// CIRABIRewriteContext. /// /// Direct: the value passes in register(s). A coercion is forwarded in the -/// three cases where the value has to be rebuilt on the wire: an aggregate +/// four cases where the value has to be rebuilt on the wire: an aggregate /// unpacked into the register(s) holding it, a scalar too wide for one register -/// split into a tuple of them, and a scalar the classifier widens to fill its -/// eightbyte. getDirect keeps canFlatten set so the rewriter can split a -/// multi-field coerced struct into individual wire arguments. Any other scalar -/// passes in its natural CIR type, which a null coercion denotes. A coercion -/// this bridge cannot represent yields std::nullopt so the caller reports NYI -/// rather than silently passing the value unchanged. +/// split into a tuple of them, a scalar the classifier widens to fill its +/// eightbyte, and a value whose live bytes start partway into its storage +/// because a leading eightbyte holds no field (getDirectOffset). getDirect +/// keeps canFlatten set so the rewriter can split a multi-field coerced +/// struct into individual wire arguments. Any other scalar passes in its +/// natural CIR type, which a null coercion denotes. A coercion this bridge +/// cannot represent yields std::nullopt so the caller reports NYI rather than +/// silently passing the value unchanged. /// /// Extend: bool or a sub-register integer needs a signext/zeroext attribute. /// The x86_64 classifier (llvm/lib/ABI/Targets/X86.cpp) only returns Extend @@ -495,11 +497,7 @@ static std::optional<ArgClassification> convertABIArgInfo(const llvm::abi::ArgInfo &info, MLIRContext *ctx, mlir::Type origTy) { if (info.isDirect()) { - // The rewriter reads a coercion from the start of the value's storage, so a - // classification naming an offset into it has no representation here. The - // classifier names one when the low eightbyte holds no field. - if (info.getDirectOffset()) - return std::nullopt; + unsigned offset = info.getDirectOffset(); // The classifier names a coerce type even where it matches the natural // type, so a non-null coerce does not by itself mean a rewrite is needed. const llvm::abi::Type *coerceAbi = info.getCoerceToType(); @@ -510,6 +508,10 @@ convertABIArgInfo(const llvm::abi::ArgInfo &info, MLIRContext *ctx, coerceAbi && isa_and_present<cir::ComplexType, cir::VectorType>(origTy); bool coerceIsRegisterTuple = isa_and_present<llvm::abi::RecordType>(coerceAbi); + // A tuple coercion cannot be read at an offset, so refuse the pair rather + // than emit a read of the wrong bytes. + if (offset && coerceIsRegisterTuple) + return std::nullopt; // Compare widths rather than identity: a coerce no wider than the natural // type carries the same value and needs no rewrite. auto origInt = dyn_cast_if_present<cir::IntType>(origTy); @@ -520,19 +522,27 @@ convertABIArgInfo(const llvm::abi::ArgInfo &info, MLIRContext *ctx, coerceInt->getSizeInBits().getFixedValue() > origInt.getWidth(); // Leaving the rest alone also avoids a lossy round trip: abiTypeToCIR // drops the LongDoubleType wrapper and a pointer's pointee, so comparing a - // scalar against its own coerce would report a difference that is not one. - if (!isAggregate && !comparesAgainstCoerce && !coerceIsRegisterTuple && - !coerceWidensScalar) + // scalar against its own coerce would report a difference that is not + // one. Passing a value through reads it from byte 0, so one living at + // an offset cannot take this path. + if (!offset && !isAggregate && !comparesAgainstCoerce && + !coerceIsRegisterTuple && !coerceWidensScalar) return ArgClassification::getDirect(nullptr); mlir::Type coerced = abiTypeToCIR(coerceAbi, ctx); if (!coerced) return std::nullopt; + // An offset would be lost here, since a coercion to the value's own type + // is indistinguishable from no coercion at all. + if (offset && coerced == origTy) + return std::nullopt; // Coercing a value to the type it already has would add a memory round // trip for nothing. if (comparesAgainstCoerce && coerced == origTy) return ArgClassification::getDirect(nullptr); - return ArgClassification::getDirect(coerced); + return ArgClassification::getDirect(coerced, offset); } + // An extended value is always read from byte 0 of its own storage, so + // there is no offset to honor here. if (info.isExtend()) { if (isa_and_present<cir::BoolType>(origTy)) return ArgClassification::getExtend(nullptr, info.isSignExt()); diff --git a/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp b/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp index 8cf332bbbefa3..91efb28625f0f 100644 --- a/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp +++ b/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp @@ -311,14 +311,21 @@ static uint64_t coercionByteSize(mlir::Type ty, const mlir::DataLayout &dl) { /// dominate every use of the coerced value and must be a block that ends up /// inside the enclosing function's entry block after any later outlining. /// +/// \p offset is where the coerced value sits within the larger of the two +/// types, non-zero when the ABI passes a value in a register read from +/// partway into its storage. The coerced type must be the smaller of the +/// two, so one that can exceed the value it coerces, such as a multi-field +/// register tuple, must not be given an offset. +/// /// Any operations the helper creates are appended to \p createdOps so the /// caller can pass them to replaceAllUsesExcept and avoid clobbering the /// store's value operand when later rewiring the source value. -mlir::Value -emitCoercionToMemory(mlir::OpBuilder &builder, mlir::Location loc, - mlir::Type dstTy, mlir::Value src, mlir::Block *slotBlock, - const mlir::DataLayout &dl, - SmallPtrSetImpl<mlir::Operation *> &createdOps) { +mlir::Value emitCoercionToMemory(mlir::OpBuilder &builder, mlir::Location loc, + mlir::Type dstTy, mlir::Value src, + mlir::Block *slotBlock, + const mlir::DataLayout &dl, + SmallPtrSetImpl<mlir::Operation *> &createdOps, + unsigned offset = 0) { mlir::Type srcTy = src.getType(); assert(srcTy != dstTy && "emitCoercion callers must pre-check that the types differ"); @@ -330,6 +337,19 @@ emitCoercionToMemory(mlir::OpBuilder &builder, mlir::Location loc, ? srcTy : dstTy; + // Sizes are compared two ways on purpose: relative size in + // coercionByteSize terms, which is how slotTy was picked, and capacity in + // getTypeSize terms, which is what the alloca is given. + [[maybe_unused]] mlir::Type coercedTy = slotTy == srcTy ? dstTy : srcTy; + assert((offset == 0 || + coercionByteSize(coercedTy, dl) < coercionByteSize(slotTy, dl)) && + "a direct offset must land on the coerced side, the smaller one"); + assert((offset == 0 || + offset + dl.getTypeSize(coercedTy) <= dl.getTypeSize(slotTy)) && + "coerce slot too small for offset access"); + assert((offset == 0 || offset % dl.getTypeABIAlignment(coercedTy) == 0) && + "a direct offset must be aligned for the coerced access"); + auto slotPtrTy = cir::PointerType::get(slotTy); auto srcPtrTy = cir::PointerType::get(srcTy); auto dstPtrTy = cir::PointerType::get(dstTy); @@ -344,25 +364,42 @@ emitCoercionToMemory(mlir::OpBuilder &builder, mlir::Location loc, } createdOps.insert(alloca); + // The alloca already has slotTy, so asking for that type returns it + // unchanged. Any other type is reached by a bitcast, preceded by a byte + // stride when the coerced value lives at an offset. + auto slotView = [&](mlir::Type wantTy, + cir::PointerType wantPtrTy) -> mlir::Value { + if (wantTy == slotTy) + return alloca; + mlir::Value base = alloca; + if (offset != 0) { + auto u8Ty = + cir::IntType::get(builder.getContext(), 8, /*isSigned=*/false); + auto u8PtrTy = cir::PointerType::get(u8Ty); + auto u8Base = cir::CastOp::create(builder, loc, u8PtrTy, + cir::CastKind::bitcast, alloca); + createdOps.insert(u8Base); + auto strideTy = + cir::IntType::get(builder.getContext(), 64, /*isSigned=*/true); + auto strideVal = cir::ConstantOp::create( + builder, loc, cir::IntAttr::get(strideTy, offset)); + createdOps.insert(strideVal); + base = cir::PtrStrideOp::create(builder, loc, u8PtrTy, u8Base, strideVal); + createdOps.insert(base.getDefiningOp()); + } + auto cast = cir::CastOp::create(builder, loc, wantPtrTy, + cir::CastKind::bitcast, base); + createdOps.insert(cast); + return cast; + }; + // Store through a source-typed view of the slot. - mlir::Value srcSlot = alloca; - if (slotTy != srcTy) { - auto srcCast = cir::CastOp::create(builder, loc, srcPtrTy, - cir::CastKind::bitcast, alloca); - createdOps.insert(srcCast); - srcSlot = srcCast; - } + mlir::Value srcSlot = slotView(srcTy, srcPtrTy); auto store = cir::StoreOp::create(builder, loc, src, srcSlot); createdOps.insert(store); // Return a destination-typed view of the slot. - if (slotTy != dstTy) { - auto dstCast = cir::CastOp::create(builder, loc, dstPtrTy, - cir::CastKind::bitcast, alloca); - createdOps.insert(dstCast); - return dstCast; - } - return alloca; + return slotView(dstTy, dstPtrTy); } /// Coerce \p src to type \p dstTy by going through memory and load the whole @@ -371,9 +408,10 @@ emitCoercionToMemory(mlir::OpBuilder &builder, mlir::Location loc, mlir::Value emitCoercion(mlir::OpBuilder &builder, mlir::Location loc, mlir::Type dstTy, mlir::Value src, mlir::Block *slotBlock, const mlir::DataLayout &dl, - SmallPtrSetImpl<mlir::Operation *> &createdOps) { - mlir::Value dstSlot = - emitCoercionToMemory(builder, loc, dstTy, src, slotBlock, dl, createdOps); + SmallPtrSetImpl<mlir::Operation *> &createdOps, + unsigned offset = 0) { + mlir::Value dstSlot = emitCoercionToMemory(builder, loc, dstTy, src, + slotBlock, dl, createdOps, offset); auto load = cir::LoadOp::create(builder, loc, dstSlot); createdOps.insert(load); return load; @@ -383,9 +421,10 @@ mlir::Value emitCoercion(mlir::OpBuilder &builder, mlir::Location loc, /// (e.g. call-site coercion where we don't replaceAllUsesExcept). mlir::Value emitCoercion(mlir::OpBuilder &builder, mlir::Location loc, mlir::Type dstTy, mlir::Value src, - mlir::Block *slotBlock, const mlir::DataLayout &dl) { + mlir::Block *slotBlock, const mlir::DataLayout &dl, + unsigned offset = 0) { SmallPtrSet<mlir::Operation *, 4> ignored; - return emitCoercion(builder, loc, dstTy, src, slotBlock, dl, ignored); + return emitCoercion(builder, loc, dstTy, src, slotBlock, dl, ignored, offset); } /// The block a coercion slot's alloca belongs at the start of. @@ -414,8 +453,8 @@ mlir::Block *coercionSlotBlock(mlir::Operation *op) { /// new (coerced) return type. void insertReturnCoercion(mlir::FunctionOpInterface funcOp, mlir::Type origRetTy, mlir::Type coercedRetTy, - mlir::OpBuilder &builder, - const mlir::DataLayout &dl) { + mlir::OpBuilder &builder, const mlir::DataLayout &dl, + unsigned offset) { SmallVector<cir::ReturnOp> returns; funcOp.walk([&](cir::ReturnOp r) { returns.push_back(r); }); for (cir::ReturnOp r : returns) { @@ -427,7 +466,7 @@ void insertReturnCoercion(mlir::FunctionOpInterface funcOp, builder.setInsertionPoint(r); mlir::Value coerced = emitCoercion(builder, r.getLoc(), coercedRetTy, origVal, - &funcOp->getRegion(0).front(), dl); + &funcOp->getRegion(0).front(), dl, offset); r->setOperand(0, coerced); } } @@ -650,6 +689,9 @@ void insertArgCoercion(mlir::FunctionOpInterface funcOp, Value finalVal = flatLoaded; if (origTy != flatTy) { SmallPtrSet<Operation *, 4> coercionOps; + assert(!ac.directOffset && + "each field is read from slot offset 0 here, so a flattened " + "coercion cannot honor a direct offset"); finalVal = emitCoercion(builder, loc, origTy, flatLoaded, &entry, dl, coercionOps); flattenOps.insert(coercionOps.begin(), coercionOps.end()); @@ -674,8 +716,9 @@ void insertArgCoercion(mlir::FunctionOpInterface funcOp, builder.setInsertionPointToStart(&entry); SmallPtrSet<mlir::Operation *, 4> coercionOps; - mlir::Value adapted = emitCoercion(builder, funcOp.getLoc(), oldArgTy, - blockArg, &entry, dl, coercionOps); + mlir::Value adapted = + emitCoercion(builder, funcOp.getLoc(), oldArgTy, blockArg, &entry, dl, + coercionOps, ac.directOffset); // Replace blockArg uses with the adapted value, except inside the // helper ops we just created. This is critical: the StoreOp's value @@ -1062,7 +1105,7 @@ mlir::LogicalResult CIRABIRewriteContext::rewriteFunctionDefinition( if (fc.returnInfo.kind == ArgKind::Direct && fc.returnInfo.coercedType && !oldResultTypes.empty() && fc.returnInfo.coercedType != origRetTy) insertReturnCoercion(funcOp, origRetTy, fc.returnInfo.coercedType, - builder, dl); + builder, dl, fc.returnInfo.directOffset); mlir::Block &entry = body.front(); @@ -1224,6 +1267,9 @@ CIRABIRewriteContext::rewriteCallSite(mlir::Operation *callOp, // alloca when possible). if (arg.getType() != flatTy) { SmallPtrSet<mlir::Operation *, 4> coercionOps; + assert(!ac.directOffset && + "each field is read from slot offset 0 here, so a flattened " + "coercion cannot honor a direct offset"); mlir::Value coercedPtr = emitCoercionToMemory( builder, call.getLoc(), flatTy, arg, slotBlock, dl, coercionOps); for (auto [f, fieldTy] : llvm::enumerate(flatTy.getMembers())) { @@ -1249,7 +1295,7 @@ CIRABIRewriteContext::rewriteCallSite(mlir::Operation *callOp, } else if (ac.kind == ArgKind::Direct && ac.coercedType && arg.getType() != ac.coercedType) { arg = emitCoercion(builder, call.getLoc(), ac.coercedType, arg, slotBlock, - dl); + dl, ac.directOffset); newArgs.push_back(arg); } else if (ac.kind == ArgKind::Indirect) { // byval hands the callee its own copy. byref must name the caller's @@ -1317,8 +1363,9 @@ CIRABIRewriteContext::rewriteCallSite(mlir::Operation *callOp, // emit a coercion back to the original type for the call's existing uses. if (returnNeedsCoercion) { builder.setInsertionPointAfter(newCall); - mlir::Value coercedBack = emitCoercion(builder, call.getLoc(), origRetTy, - newCall.getResult(), slotBlock, dl); + mlir::Value coercedBack = + emitCoercion(builder, call.getLoc(), origRetTy, newCall.getResult(), + slotBlock, dl, fc.returnInfo.directOffset); call.getResult().replaceAllUsesWith(coercedBack); } diff --git a/clang/test/CIR/CodeGen/call-conv-lowering-x86_64-empty.cpp b/clang/test/CIR/CodeGen/call-conv-lowering-x86_64-empty.cpp index 30999935ef9d0..8fa7fefec41f6 100644 --- a/clang/test/CIR/CodeGen/call-conv-lowering-x86_64-empty.cpp +++ b/clang/test/CIR/CodeGen/call-conv-lowering-x86_64-empty.cpp @@ -24,6 +24,7 @@ struct EmptySecond { long a; Empty e; }; struct EmptySSE { double a; Empty e; }; struct FloatEmpty { float a; Empty e; }; struct FloatEmptyFirst { Empty e; float a; }; +struct HiWord { Empty e; long hi; }; struct alignas(32) Big32 {}; union UBits { unsigned : 3; }; union UNone {}; @@ -145,6 +146,36 @@ float takeFloatEmptyFirst(FloatEmptyFirst v) { return v.a; } // CIR: cir.func {{.*}}@_Z19takeFloatEmptyFirst15FloatEmptyFirst(%arg0: !cir.double {{.*}}) -> (!cir.float // LLVM: define dso_local noundef float @_Z19takeFloatEmptyFirst15FloatEmptyFirst(double %{{[^,]+}}) +// Here the empty member owns eightbyte 0 rather than eightbyte 1, so NoClass +// cannot just be dropped: the coercion has to start at byte 8. +long takeHiWord(HiWord v) { return v.hi; } + +// CIR: cir.func {{.*}}@_Z10takeHiWord6HiWord(%arg0: !s64i {{.*}}) -> (!s64i +// LLVM: define dso_local noundef i64 @_Z10takeHiWord6HiWord(i64 %{{[^,]+}}) +// LLVM: %{{.+}} = getelementptr{{( inbounds)?}} i8, ptr %{{.+}}, i64 8 +// LLVM: store i64 %{{.+}}, ptr %{{.+}}, align 8 + +// The same offset on the return side. +HiWord giveHiWord(long hi) { + HiWord w; + w.hi = hi; + return w; +} + +// CIR: cir.func {{.*}}@_Z10giveHiWordl(%arg0: !s64i {llvm.noundef} {{.*}}) -> !s64i +// LLVM: define dso_local i64 @_Z10giveHiWordl(i64 noundef %{{[^,]+}}) +// LLVM: %[[RGEP:.+]] = getelementptr{{( inbounds)?}} i8, ptr %{{.+}}, i64 8 +// LLVM: %[[RVAL:.+]] = load i64, ptr %[[RGEP]], align 8 +// LLVM: ret i64 %[[RVAL]] + +// The caller's own signature is unaffected by the callee's coercion. +long callerHiWord(long hi) { + return takeHiWord(giveHiWord(hi)); +} + +// CIR: cir.func {{.*}}@_Z12callerHiWordl(%arg0: !s64i {{.*}}) -> (!s64i +// LLVM: define dso_local noundef i64 @_Z12callerHiWordl(i64 noundef %{{[^,]+}}) + // Past two eightbytes SysV says memory whatever the content, so an empty class // this size is passed indirectly at its declared alignment. int takeBig32(Big32 v, int k) { return k; } diff --git a/clang/test/CIR/Transforms/abi-lowering/x86_64-aggregate-nyi.cir b/clang/test/CIR/Transforms/abi-lowering/x86_64-aggregate-nyi.cir index 7da3a76abc494..b9e2d795e70b4 100644 --- a/clang/test/CIR/Transforms/abi-lowering/x86_64-aggregate-nyi.cir +++ b/clang/test/CIR/Transforms/abi-lowering/x86_64-aggregate-nyi.cir @@ -33,7 +33,6 @@ !rec_HoldsAllPad = !cir.struct<"HoldsAllPad" {data !s32i, empty !rec_E}> !rec_EBits = !cir.struct<"EBits" {empty !u32i}> !rec_HoldsEmptyBits = !cir.struct<"HoldsEmptyBits" {empty !rec_EBits, data !s32i}> -!rec_LeadPad = !cir.struct<"LeadPad" {pad !cir.array<!u8i x 8>, data !s32i}> !rec_Incomplete = !cir.struct<"Incomplete" incomplete> module attributes { @@ -196,15 +195,6 @@ module attributes { // CHECK: not yet implemented for type '!cir.struct<"HoldsEmptyBits" {empty - // With no field in the low eightbyte the classifier coerces the high one and - // names an offset of 8 into the record, which the rewriter cannot express: it - // reads a coercion from offset 0, so the argument would land in the padding. - cir.func @take_lead_pad(%arg0: !rec_LeadPad) { - cir.return - } - - // CHECK: not yet implemented for the ABI coercion of type '!cir.struct<"LeadPad" - // A scalable vector has no size the eightbyte rules can read. cir.func @take_scalable(%arg0: !cir.vector<[4] x !cir.float>) { cir.return diff --git a/clang/test/CIR/Transforms/abi-lowering/x86_64-struct-direct-offset.cir b/clang/test/CIR/Transforms/abi-lowering/x86_64-struct-direct-offset.cir new file mode 100644 index 0000000000000..07132c8c308fc --- /dev/null +++ b/clang/test/CIR/Transforms/abi-lowering/x86_64-struct-direct-offset.cir @@ -0,0 +1,123 @@ +// RUN: cir-opt %s -cir-call-conv-lowering=target=x86_64 | FileCheck %s +// RUN: cir-opt %s -cir-call-conv-lowering=target=x86_64 -cir-to-llvm -o - 2>/dev/null \ +// RUN: | mlir-translate -mlir-to-llvmir --allow-unregistered-dialect \ +// RUN: | FileCheck %s --check-prefix=LLVM + +!u8i = !cir.int<u, 8> +!s32i = !cir.int<s, 32> +!s64i = !cir.int<s, 64> +!rec_Empty = !cir.struct<"Empty" {pad !u8i}> +!rec_HiWord = !cir.struct<"HiWord" {data !rec_Empty, data !s64i}> +!rec_LeadPad = !cir.struct<"LeadPad" {pad !cir.array<!u8i x 8>, data !s32i}> + +module attributes { + cir.triple = "x86_64-unknown-linux-gnu", + dlti.dl_spec = #dlti.dl_spec< + #dlti.dl_entry<i8, dense<8>: vector<2xi64>>, + #dlti.dl_entry<i32, dense<32>: vector<2xi64>>, + #dlti.dl_entry<i64, dense<64>: vector<2xi64>>> +} { + + // HiWord is 16 bytes: a leading ABI-empty byte, then an i64 at offset 8. + // The low eightbyte carries no field (NO_CLASS), so the classifier passes + // the value in one register taken from byte 8. + cir.func @take_hiword(%arg0: !rec_HiWord) -> !s64i { + %0 = cir.alloca "h" align(8) : !cir.ptr<!rec_HiWord> + cir.store %arg0, %0 : !rec_HiWord, !cir.ptr<!rec_HiWord> + %1 = cir.get_member %0[1] {name = "hi"} : !cir.ptr<!rec_HiWord> -> !cir.ptr<!s64i> + %2 = cir.load %1 : !cir.ptr<!s64i>, !s64i + cir.return %2 : !s64i + } + + // CHECK-LABEL: cir.func{{.*}} @take_hiword(%arg0: !s64i) -> !s64i + // CHECK: %[[SLOT:.*]] = cir.alloca "coerce" {{.*}} : !cir.ptr<!rec_HiWord> + // CHECK: %[[U8:.*]] = cir.cast bitcast %[[SLOT]] : !cir.ptr<!rec_HiWord> -> !cir.ptr<!u8i> + // CHECK: %[[OFF:.*]] = cir.const #cir.int<8> : !s64i + // CHECK: %[[GEP:.*]] = cir.ptr_stride %[[U8]], %[[OFF]] : (!cir.ptr<!u8i>, !s64i) -> !cir.ptr<!u8i> + // CHECK: %[[CAST:.*]] = cir.cast bitcast %[[GEP]] : !cir.ptr<!u8i> -> !cir.ptr<!s64i> + // CHECK: cir.store %arg0, %[[CAST]] : !s64i, !cir.ptr<!s64i> + // CHECK: %{{.*}} = cir.load %[[SLOT]] : !cir.ptr<!rec_HiWord>, !rec_HiWord + + // The return value is likewise coerced from byte 8 of the local aggregate. + cir.func @give_hiword() -> !rec_HiWord { + %0 = cir.alloca "h" align(8) : !cir.ptr<!rec_HiWord> + %1 = cir.load %0 : !cir.ptr<!rec_HiWord>, !rec_HiWord + cir.return %1 : !rec_HiWord + } + + // CHECK-LABEL: cir.func{{.*}} @give_hiword() -> !s64i + // CHECK: %[[RSLOT:.*]] = cir.alloca "coerce" {{.*}} : !cir.ptr<!rec_HiWord> + // CHECK: cir.store %{{.*}}, %[[RSLOT]] : !rec_HiWord, !cir.ptr<!rec_HiWord> + // CHECK: %[[RU8:.*]] = cir.cast bitcast %[[RSLOT]] : !cir.ptr<!rec_HiWord> -> !cir.ptr<!u8i> + // CHECK: %[[ROFF:.*]] = cir.const #cir.int<8> : !s64i + // CHECK: %[[RGEP:.*]] = cir.ptr_stride %[[RU8]], %[[ROFF]] : (!cir.ptr<!u8i>, !s64i) -> !cir.ptr<!u8i> + // CHECK: %[[RCAST:.*]] = cir.cast bitcast %[[RGEP]] : !cir.ptr<!u8i> -> !cir.ptr<!s64i> + // CHECK: %[[RVAL:.*]] = cir.load %[[RCAST]] : !cir.ptr<!s64i>, !s64i + // CHECK: cir.return %[[RVAL]] : !s64i + + // The caller reconstructs the aggregate from the callee's i64 result, so + // the received register is written back to byte 8 of a slot. + cir.func @call_hiword() -> !s64i { + %0 = cir.call @give_hiword() : () -> !rec_HiWord + %1 = cir.alloca "r" align(8) : !cir.ptr<!rec_HiWord> + cir.store %0, %1 : !rec_HiWord, !cir.ptr<!rec_HiWord> + %2 = cir.get_member %1[1] {name = "hi"} : !cir.ptr<!rec_HiWord> -> !cir.ptr<!s64i> + %3 = cir.load %2 : !cir.ptr<!s64i>, !s64i + cir.return %3 : !s64i + } + + // CHECK-LABEL: cir.func{{.*}} @call_hiword() -> !s64i + // CHECK: %[[CSLOT:.*]] = cir.alloca "coerce" {{.*}} : !cir.ptr<!rec_HiWord> + // CHECK: %[[CRET:.*]] = cir.call @give_hiword() : () -> !s64i + // CHECK: %[[CU8:.*]] = cir.cast bitcast %[[CSLOT]] : !cir.ptr<!rec_HiWord> -> !cir.ptr<!u8i> + // CHECK: %[[COFF:.*]] = cir.const #cir.int<8> : !s64i + // CHECK: %[[CGEP:.*]] = cir.ptr_stride %[[CU8]], %[[COFF]] : (!cir.ptr<!u8i>, !s64i) -> !cir.ptr<!u8i> + // CHECK: %[[CCAST:.*]] = cir.cast bitcast %[[CGEP]] : !cir.ptr<!u8i> -> !cir.ptr<!s64i> + // CHECK: cir.store %[[CRET]], %[[CCAST]] : !s64i, !cir.ptr<!s64i> + // CHECK: %{{.*}} = cir.load %[[CSLOT]] : !cir.ptr<!rec_HiWord>, !rec_HiWord + + // Passing an aggregate argument runs the extraction direction at the call + // site: the operand's byte 8 is read out to form the register. + cir.func @pass_hiword(%arg0: !rec_HiWord) { + %0 = cir.call @take_hiword(%arg0) : (!rec_HiWord) -> !s64i + cir.return + } + + // CHECK-LABEL: cir.func{{.*}} @pass_hiword(%arg0: !s64i) + // CHECK: %[[PREC:.*]] = cir.load %{{.*}} : !cir.ptr<!rec_HiWord>, !rec_HiWord + // CHECK: %[[PSLOT:.*]] = cir.alloca "coerce" {{.*}} : !cir.ptr<!rec_HiWord> + // CHECK: cir.store %[[PREC]], %[[PSLOT]] : !rec_HiWord, !cir.ptr<!rec_HiWord> + // CHECK: %[[PU8:.*]] = cir.cast bitcast %[[PSLOT]] : !cir.ptr<!rec_HiWord> -> !cir.ptr<!u8i> + // CHECK: %[[POFF:.*]] = cir.const #cir.int<8> : !s64i + // CHECK: %[[PGEP:.*]] = cir.ptr_stride %[[PU8]], %[[POFF]] : (!cir.ptr<!u8i>, !s64i) -> !cir.ptr<!u8i> + // CHECK: %[[PCAST:.*]] = cir.cast bitcast %[[PGEP]] : !cir.ptr<!u8i> -> !cir.ptr<!s64i> + // CHECK: %[[PVAL:.*]] = cir.load %[[PCAST]] : !cir.ptr<!s64i>, !s64i + // CHECK: %{{.*}} = cir.call @take_hiword(%[[PVAL]]) : (!s64i) -> !s64i + + // The same offset-8 coercion from a leading pad field rather than a nested + // empty record. Only 4 live bytes in the high eightbyte, so i32 not i64. + cir.func @take_lead_pad(%arg0: !rec_LeadPad) -> !s32i { + %0 = cir.alloca "l" align(4) : !cir.ptr<!rec_LeadPad> + cir.store %arg0, %0 : !rec_LeadPad, !cir.ptr<!rec_LeadPad> + %1 = cir.get_member %0[1] {name = "data"} : !cir.ptr<!rec_LeadPad> -> !cir.ptr<!s32i> + %2 = cir.load %1 : !cir.ptr<!s32i>, !s32i + cir.return %2 : !s32i + } + + // CHECK-LABEL: cir.func{{.*}} @take_lead_pad(%arg0: !s32i) -> !s32i + // CHECK: %[[LSLOT:.*]] = cir.alloca "coerce" {{.*}} : !cir.ptr<!rec_LeadPad> + // CHECK: %[[LU8:.*]] = cir.cast bitcast %[[LSLOT]] : !cir.ptr<!rec_LeadPad> -> !cir.ptr<!u8i> + // CHECK: %[[LOFF:.*]] = cir.const #cir.int<8> : !s64i + // CHECK: %[[LGEP:.*]] = cir.ptr_stride %[[LU8]], %[[LOFF]] : (!cir.ptr<!u8i>, !s64i) -> !cir.ptr<!u8i> + // CHECK: %[[LCAST:.*]] = cir.cast bitcast %[[LGEP]] : !cir.ptr<!u8i> -> !cir.ptr<!s32i> + // CHECK: cir.store %arg0, %[[LCAST]] : !s32i, !cir.ptr<!s32i> + // CHECK: %{{.*}} = cir.load %[[LSLOT]] : !cir.ptr<!rec_LeadPad>, !rec_LeadPad +} + +// LLVM: define i64 @take_hiword(i64 %{{.+}}) +// LLVM: define i64 @give_hiword() +// LLVM: define i64 @call_hiword() +// LLVM: %{{.+}} = call i64 @give_hiword() +// LLVM: define void @pass_hiword(i64 %{{.+}}) +// LLVM: %{{.+}} = call i64 @take_hiword(i64 %{{.+}}) +// LLVM: define i32 @take_lead_pad(i32 %{{.+}}) diff --git a/mlir/include/mlir/ABI/ABIRewriteContext.h b/mlir/include/mlir/ABI/ABIRewriteContext.h index 8394aeccc75ce..c1c205e42b2e4 100644 --- a/mlir/include/mlir/ABI/ABIRewriteContext.h +++ b/mlir/include/mlir/ABI/ABIRewriteContext.h @@ -27,6 +27,8 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/Support/Alignment.h" +#include <cassert> + namespace mlir { namespace abi { @@ -75,6 +77,11 @@ struct ArgClassification { /// For Indirect: whether the callee gets ownership (byval). bool byVal = false; + /// For Direct with coercion: the byte offset within the original aggregate + /// at which the coerced value lives. Non-zero when the low eightbyte is + /// NO_CLASS and the value is carried in a later eightbyte (x86-64 SysV). + unsigned directOffset = 0; + /// Whether the value is passed as-is, so a rewriter can leave it alone. /// Only an uncoerced Direct qualifies. Extend counts as needing a rewrite /// even though it only adds an attribute, because the attribute changes @@ -87,13 +94,19 @@ struct ArgClassification { return kind == other.kind && coercedType == other.coercedType && indirectAlign == other.indirectAlign && signExtend == other.signExtend && canFlatten == other.canFlatten && - byVal == other.byVal; + byVal == other.byVal && directOffset == other.directOffset; } - static ArgClassification getDirect(Type coerced = nullptr) { + static ArgClassification getDirect(Type coerced = nullptr, + unsigned offset = 0) { + // isPassThrough reads only coercedType, so an offset with no coerced + // type to read at it would be silently ignored. + assert((!offset || coerced) && + "a direct offset needs a coerced type to read at it"); ArgClassification c; c.kind = ArgKind::Direct; c.coercedType = coerced; + c.directOffset = offset; return c; } >From 697007d9d5bee2b0937953d26d7d1c41c54208d6 Mon Sep 17 00:00:00 2001 From: Adam Smith <[email protected]> Date: Wed, 2 Sep 2026 15:53:08 -0500 Subject: [PATCH 2/5] Update clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp Taken verbatim. Co-authored-by: Erich Keane <[email protected]> --- .../Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp b/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp index 91efb28625f0f..23db84f67e78f 100644 --- a/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp +++ b/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp @@ -340,7 +340,7 @@ mlir::Value emitCoercionToMemory(mlir::OpBuilder &builder, mlir::Location loc, // Sizes are compared two ways on purpose: relative size in // coercionByteSize terms, which is how slotTy was picked, and capacity in // getTypeSize terms, which is what the alloca is given. - [[maybe_unused]] mlir::Type coercedTy = slotTy == srcTy ? dstTy : srcTy; + [[maybe_unused]] mlir::Type coercedTy = ((slotTy == srcTy) ? dstTy : srcTy); assert((offset == 0 || coercionByteSize(coercedTy, dl) < coercionByteSize(slotTy, dl)) && "a direct offset must land on the coerced side, the smaller one"); >From cd35d3473613e8293855d4489def57bebf20c0b6 Mon Sep 17 00:00:00 2001 From: Adam Smith <[email protected]> Date: Thu, 3 Sep 2026 07:16:49 -0700 Subject: [PATCH 3/5] [CIR][NFC] Require the coercion offset to be passed explicitly Assisted-by: Cursor / claude-opus-5 --- .../CIR/Dialect/Transforms/CallConvLoweringPass.cpp | 6 +++--- .../TargetLowering/CIRABIRewriteContext.cpp | 13 +++++++------ mlir/include/mlir/ABI/ABIRewriteContext.h | 3 +-- mlir/lib/ABI/Targets/Test/TestTarget.cpp | 10 +++++----- mlir/unittests/ABI/ABIRewriteContextTest.cpp | 8 ++++---- 5 files changed, 20 insertions(+), 20 deletions(-) diff --git a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp index 242fda30343a1..e967772802fcf 100644 --- a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp +++ b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp @@ -527,7 +527,7 @@ convertABIArgInfo(const llvm::abi::ArgInfo &info, MLIRContext *ctx, // an offset cannot take this path. if (!offset && !isAggregate && !comparesAgainstCoerce && !coerceIsRegisterTuple && !coerceWidensScalar) - return ArgClassification::getDirect(nullptr); + return ArgClassification::getDirect(/*offset=*/0); mlir::Type coerced = abiTypeToCIR(coerceAbi, ctx); if (!coerced) return std::nullopt; @@ -538,8 +538,8 @@ convertABIArgInfo(const llvm::abi::ArgInfo &info, MLIRContext *ctx, // Coercing a value to the type it already has would add a memory round // trip for nothing. if (comparesAgainstCoerce && coerced == origTy) - return ArgClassification::getDirect(nullptr); - return ArgClassification::getDirect(coerced, offset); + return ArgClassification::getDirect(/*offset=*/0); + return ArgClassification::getDirect(offset, coerced); } // An extended value is always read from byte 0 of its own storage, so // there is no offset to honor here. diff --git a/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp b/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp index 23db84f67e78f..b2e6016e14e54 100644 --- a/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp +++ b/clang/lib/CIR/Dialect/Transforms/TargetLowering/CIRABIRewriteContext.cpp @@ -325,7 +325,7 @@ mlir::Value emitCoercionToMemory(mlir::OpBuilder &builder, mlir::Location loc, mlir::Block *slotBlock, const mlir::DataLayout &dl, SmallPtrSetImpl<mlir::Operation *> &createdOps, - unsigned offset = 0) { + unsigned offset) { mlir::Type srcTy = src.getType(); assert(srcTy != dstTy && "emitCoercion callers must pre-check that the types differ"); @@ -409,7 +409,7 @@ mlir::Value emitCoercion(mlir::OpBuilder &builder, mlir::Location loc, mlir::Type dstTy, mlir::Value src, mlir::Block *slotBlock, const mlir::DataLayout &dl, SmallPtrSetImpl<mlir::Operation *> &createdOps, - unsigned offset = 0) { + unsigned offset) { mlir::Value dstSlot = emitCoercionToMemory(builder, loc, dstTy, src, slotBlock, dl, createdOps, offset); auto load = cir::LoadOp::create(builder, loc, dstSlot); @@ -422,7 +422,7 @@ mlir::Value emitCoercion(mlir::OpBuilder &builder, mlir::Location loc, mlir::Value emitCoercion(mlir::OpBuilder &builder, mlir::Location loc, mlir::Type dstTy, mlir::Value src, mlir::Block *slotBlock, const mlir::DataLayout &dl, - unsigned offset = 0) { + unsigned offset) { SmallPtrSet<mlir::Operation *, 4> ignored; return emitCoercion(builder, loc, dstTy, src, slotBlock, dl, ignored, offset); } @@ -693,7 +693,7 @@ void insertArgCoercion(mlir::FunctionOpInterface funcOp, "each field is read from slot offset 0 here, so a flattened " "coercion cannot honor a direct offset"); finalVal = emitCoercion(builder, loc, origTy, flatLoaded, &entry, dl, - coercionOps); + coercionOps, /*offset=*/0); flattenOps.insert(coercionOps.begin(), coercionOps.end()); } @@ -1270,8 +1270,9 @@ CIRABIRewriteContext::rewriteCallSite(mlir::Operation *callOp, assert(!ac.directOffset && "each field is read from slot offset 0 here, so a flattened " "coercion cannot honor a direct offset"); - mlir::Value coercedPtr = emitCoercionToMemory( - builder, call.getLoc(), flatTy, arg, slotBlock, dl, coercionOps); + mlir::Value coercedPtr = + emitCoercionToMemory(builder, call.getLoc(), flatTy, arg, slotBlock, + dl, coercionOps, /*offset=*/0); for (auto [f, fieldTy] : llvm::enumerate(flatTy.getMembers())) { mlir::Type fieldPtrTy = cir::PointerType::get(fieldTy); auto fieldPtr = diff --git a/mlir/include/mlir/ABI/ABIRewriteContext.h b/mlir/include/mlir/ABI/ABIRewriteContext.h index c1c205e42b2e4..0004bb77b6801 100644 --- a/mlir/include/mlir/ABI/ABIRewriteContext.h +++ b/mlir/include/mlir/ABI/ABIRewriteContext.h @@ -97,8 +97,7 @@ struct ArgClassification { byVal == other.byVal && directOffset == other.directOffset; } - static ArgClassification getDirect(Type coerced = nullptr, - unsigned offset = 0) { + static ArgClassification getDirect(unsigned offset, Type coerced = nullptr) { // isPassThrough reads only coercedType, so an offset with no coerced // type to read at it would be silently ignored. assert((!offset || coerced) && diff --git a/mlir/lib/ABI/Targets/Test/TestTarget.cpp b/mlir/lib/ABI/Targets/Test/TestTarget.cpp index 0cfdb2eb3fdb0..66a6a29bd3e6f 100644 --- a/mlir/lib/ABI/Targets/Test/TestTarget.cpp +++ b/mlir/lib/ABI/Targets/Test/TestTarget.cpp @@ -69,7 +69,7 @@ ArgClassification classifyOne(Type type, const DataLayout &dl) { Type i32Ty = IntegerType::get(type.getContext(), ExtendBelowBits); return ArgClassification::getExtend(i32Ty, /*signExt=*/intTy.isSigned()); } - return ArgClassification::getDirect(); + return ArgClassification::getDirect(/*offset=*/0); } if (auto indexTy = dyn_cast<IndexType>(type)) { @@ -78,11 +78,11 @@ ArgClassification classifyOne(Type type, const DataLayout &dl) { Type i32Ty = IntegerType::get(type.getContext(), ExtendBelowBits); return ArgClassification::getExtend(i32Ty, /*signExt=*/true); } - return ArgClassification::getDirect(); + return ArgClassification::getDirect(/*offset=*/0); } if (isa<FloatType, VectorType, MemRefType>(type)) - return ArgClassification::getDirect(); + return ArgClassification::getDirect(/*offset=*/0); // For dialect-specific types: query DataLayout via // DataLayoutTypeInterface. Types that don't implement the interface @@ -99,7 +99,7 @@ ArgClassification classifyOne(Type type, const DataLayout &dl) { uint64_t sizeInBytes = (sizeInBits.getFixedValue() + 7) / 8; if (sizeInBytes <= IndirectCutoffBytes) - return ArgClassification::getDirect(); + return ArgClassification::getDirect(/*offset=*/0); uint64_t alignBytes = dl.getTypeABIAlignment(type); return ArgClassification::getIndirect(llvm::Align(alignBytes), @@ -161,7 +161,7 @@ parseOne(DictionaryAttr argDict, function_ref<InFlightDiagnostic()> emitError) { Type coerced; if (auto t = argDict.getAs<TypeAttr>("coerced_type")) coerced = t.getValue(); - auto c = ArgClassification::getDirect(coerced); + auto c = ArgClassification::getDirect(/*offset=*/0, coerced); if (auto cf = argDict.getAs<BoolAttr>("can_flatten")) c.canFlatten = cf.getValue(); return c; diff --git a/mlir/unittests/ABI/ABIRewriteContextTest.cpp b/mlir/unittests/ABI/ABIRewriteContextTest.cpp index 59a5307225e0d..e76c5f3821528 100644 --- a/mlir/unittests/ABI/ABIRewriteContextTest.cpp +++ b/mlir/unittests/ABI/ABIRewriteContextTest.cpp @@ -36,7 +36,7 @@ TEST(ABIRewriteContextTest, MockCanBeConstructedAndDestroyed) { } TEST(ABIRewriteContextTest, ArgClassificationDirect) { - auto c = ArgClassification::getDirect(); + auto c = ArgClassification::getDirect(/*offset=*/0); EXPECT_EQ(c.kind, ArgKind::Direct); EXPECT_EQ(c.coercedType, nullptr); EXPECT_TRUE(c.canFlatten); @@ -45,7 +45,7 @@ TEST(ABIRewriteContextTest, ArgClassificationDirect) { TEST(ABIRewriteContextTest, ArgClassificationDirectWithType) { MLIRContext mlirCtx; auto i32 = IntegerType::get(&mlirCtx, 32); - auto c = ArgClassification::getDirect(i32); + auto c = ArgClassification::getDirect(/*offset=*/0, i32); EXPECT_EQ(c.kind, ArgKind::Direct); EXPECT_EQ(c.coercedType, i32); } @@ -84,8 +84,8 @@ TEST(ABIRewriteContextTest, ArgClassificationExtend) { TEST(ABIRewriteContextTest, FunctionClassificationHoldsReturnAndArgs) { FunctionClassification fc; - fc.returnInfo = ArgClassification::getDirect(); - fc.argInfos.push_back(ArgClassification::getDirect()); + fc.returnInfo = ArgClassification::getDirect(/*offset=*/0); + fc.argInfos.push_back(ArgClassification::getDirect(/*offset=*/0)); fc.argInfos.push_back(ArgClassification::getIndirect(llvm::Align(8), true)); fc.argInfos.push_back(ArgClassification::getIgnore()); >From bbf7c29d6dc00f12baee4c31753d445766080630 Mon Sep 17 00:00:00 2001 From: Adam Smith <[email protected]> Date: Thu, 3 Sep 2026 07:16:49 -0700 Subject: [PATCH 4/5] [CIR] Enable callconv lowering in the x86_64 C++ argument test Two cases here hit the coercion NYI this branch removes. The pass now runs on the file. The `LLVM` lines for those two stopped at the open paren and passed whatever the pass did with the argument. Both prefixes now pin the coerced parameter and the byte-8 access. Assisted-by: Cursor / claude-opus-5 --- .../test/CIR/CodeGenCXX/x86_64-arguments.cpp | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/clang/test/CIR/CodeGenCXX/x86_64-arguments.cpp b/clang/test/CIR/CodeGenCXX/x86_64-arguments.cpp index 7270c36df7df6..acd6eb6a5264c 100644 --- a/clang/test/CIR/CodeGenCXX/x86_64-arguments.cpp +++ b/clang/test/CIR/CodeGenCXX/x86_64-arguments.cpp @@ -1,9 +1,6 @@ -// TODO(cir): drop -fno-clangir-call-conv-lowering once CallConvLowering -// supports parameters of an empty or tag class and padded, packed, and -// over-aligned record shapes. -// RUN: %clang_cc1 -no-enable-noundef-analysis -triple x86_64-unknown-unknown -fclangir -fno-clangir-call-conv-lowering -emit-cir %s -o %t.cir +// RUN: %clang_cc1 -no-enable-noundef-analysis -triple x86_64-unknown-unknown -fclangir -emit-cir %s -o %t.cir // RUN: FileCheck --check-prefix=CIR --input-file=%t.cir %s -// RUN: %clang_cc1 -no-enable-noundef-analysis -triple x86_64-unknown-unknown -fclangir -fno-clangir-call-conv-lowering -emit-llvm %s -o %t-cir.ll +// RUN: %clang_cc1 -no-enable-noundef-analysis -triple x86_64-unknown-unknown -fclangir -emit-llvm %s -o %t-cir.ll // RUN: FileCheck --check-prefix=LLVM --input-file=%t-cir.ll %s // RUN: %clang_cc1 -no-enable-noundef-analysis -triple x86_64-unknown-unknown -emit-llvm %s -o %t.ll // RUN: FileCheck --check-prefix=OGCG --input-file=%t.ll %s @@ -44,9 +41,17 @@ struct s3_0 {}; struct s3_1 { struct s3_0 a; long b; }; void f3(struct s3_1 x) {} +// The empty member owns eightbyte 0, so the live long is the second eightbyte +// and the coercion starts at byte 8. // CIR-LABEL: cir.func {{.*}} @_Z2f34s3_1 -// LLVM-LABEL: define {{.*}} void @_Z2f34s3_1( +// LLVM-LABEL: define {{.*}} void @_Z2f34s3_1(i64 %{{[^,)]+}}) +// LLVM: %[[SLOT:.+]] = alloca %struct.s3_1, align 8 +// LLVM: %[[HI:.+]] = getelementptr{{( inbounds)?}} i8, ptr %[[SLOT]], i64 8 +// LLVM: store i64 %{{.+}}, ptr %[[HI]], align 8 // OGCG-LABEL: define {{.*}} void @_Z2f34s3_1(i64 %x.coerce) +// OGCG: %[[SLOT:.+]] = alloca %struct.s3_1, align 8 +// OGCG: %[[HI:.+]] = getelementptr{{( inbounds)?}} i8, ptr %[[SLOT]], i64 8 +// OGCG: store i64 %x.coerce, ptr %[[HI]], align 8 // Member data pointer and member function pointer. struct s4 {}; @@ -121,9 +126,16 @@ namespace PR5179 { const void *bar(B2 b2) { return b2.b1.pa; } } +// The empty base owns eightbyte 0, so the pointer is read from byte 8. // CIR-LABEL: cir.func {{.*}} @_ZN6PR51793barENS_2B2E -// LLVM-LABEL: define {{.*}} @_ZN6PR51793barENS_2B2E( +// LLVM-LABEL: define {{.*}} ptr @_ZN6PR51793barENS_2B2E(ptr %{{[^,)]+}}) +// LLVM: %[[SLOT:.+]] = alloca %"struct.PR5179::B2", align 8 +// LLVM: %[[PA:.+]] = getelementptr{{( inbounds)?}} i8, ptr %[[SLOT]], i64 8 +// LLVM: store ptr %{{.+}}, ptr %[[PA]], align 8 // OGCG-LABEL: define {{.*}} ptr @_ZN6PR51793barENS_2B2E(ptr %b2.coerce) +// OGCG: %[[SLOT:.+]] = alloca %"struct.PR5179::B2", align 8 +// OGCG: %[[PA:.+]] = getelementptr{{( inbounds)?}} i8, ptr %[[SLOT]], i64 8 +// OGCG: store ptr %b2.coerce, ptr %[[PA]], align 8 namespace test5 { struct Xbase { }; >From ba2faf1b07cc5fe836f99fb5fab9c7cd75ef173b Mon Sep 17 00:00:00 2001 From: Adam Smith <[email protected]> Date: Thu, 3 Sep 2026 08:15:55 -0700 Subject: [PATCH 5/5] [CIR][NFC] Overload getDirect instead of defaulting its arguments Assisted-by: Cursor / claude-opus-5 --- clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp | 4 ++-- mlir/include/mlir/ABI/ABIRewriteContext.h | 6 +++++- mlir/lib/ABI/Targets/Test/TestTarget.cpp | 8 ++++---- mlir/unittests/ABI/ABIRewriteContextTest.cpp | 6 +++--- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp index e967772802fcf..c1ebd17e1e6f0 100644 --- a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp +++ b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp @@ -527,7 +527,7 @@ convertABIArgInfo(const llvm::abi::ArgInfo &info, MLIRContext *ctx, // an offset cannot take this path. if (!offset && !isAggregate && !comparesAgainstCoerce && !coerceIsRegisterTuple && !coerceWidensScalar) - return ArgClassification::getDirect(/*offset=*/0); + return ArgClassification::getDirect(); mlir::Type coerced = abiTypeToCIR(coerceAbi, ctx); if (!coerced) return std::nullopt; @@ -538,7 +538,7 @@ convertABIArgInfo(const llvm::abi::ArgInfo &info, MLIRContext *ctx, // Coercing a value to the type it already has would add a memory round // trip for nothing. if (comparesAgainstCoerce && coerced == origTy) - return ArgClassification::getDirect(/*offset=*/0); + return ArgClassification::getDirect(); return ArgClassification::getDirect(offset, coerced); } // An extended value is always read from byte 0 of its own storage, so diff --git a/mlir/include/mlir/ABI/ABIRewriteContext.h b/mlir/include/mlir/ABI/ABIRewriteContext.h index 0004bb77b6801..430ebe94bc5b6 100644 --- a/mlir/include/mlir/ABI/ABIRewriteContext.h +++ b/mlir/include/mlir/ABI/ABIRewriteContext.h @@ -97,7 +97,11 @@ struct ArgClassification { byVal == other.byVal && directOffset == other.directOffset; } - static ArgClassification getDirect(unsigned offset, Type coerced = nullptr) { + static ArgClassification getDirect() { + return getDirect(/*offset=*/0, /*coerced=*/nullptr); + } + + static ArgClassification getDirect(unsigned offset, Type coerced) { // isPassThrough reads only coercedType, so an offset with no coerced // type to read at it would be silently ignored. assert((!offset || coerced) && diff --git a/mlir/lib/ABI/Targets/Test/TestTarget.cpp b/mlir/lib/ABI/Targets/Test/TestTarget.cpp index 66a6a29bd3e6f..f36595ae52bb4 100644 --- a/mlir/lib/ABI/Targets/Test/TestTarget.cpp +++ b/mlir/lib/ABI/Targets/Test/TestTarget.cpp @@ -69,7 +69,7 @@ ArgClassification classifyOne(Type type, const DataLayout &dl) { Type i32Ty = IntegerType::get(type.getContext(), ExtendBelowBits); return ArgClassification::getExtend(i32Ty, /*signExt=*/intTy.isSigned()); } - return ArgClassification::getDirect(/*offset=*/0); + return ArgClassification::getDirect(); } if (auto indexTy = dyn_cast<IndexType>(type)) { @@ -78,11 +78,11 @@ ArgClassification classifyOne(Type type, const DataLayout &dl) { Type i32Ty = IntegerType::get(type.getContext(), ExtendBelowBits); return ArgClassification::getExtend(i32Ty, /*signExt=*/true); } - return ArgClassification::getDirect(/*offset=*/0); + return ArgClassification::getDirect(); } if (isa<FloatType, VectorType, MemRefType>(type)) - return ArgClassification::getDirect(/*offset=*/0); + return ArgClassification::getDirect(); // For dialect-specific types: query DataLayout via // DataLayoutTypeInterface. Types that don't implement the interface @@ -99,7 +99,7 @@ ArgClassification classifyOne(Type type, const DataLayout &dl) { uint64_t sizeInBytes = (sizeInBits.getFixedValue() + 7) / 8; if (sizeInBytes <= IndirectCutoffBytes) - return ArgClassification::getDirect(/*offset=*/0); + return ArgClassification::getDirect(); uint64_t alignBytes = dl.getTypeABIAlignment(type); return ArgClassification::getIndirect(llvm::Align(alignBytes), diff --git a/mlir/unittests/ABI/ABIRewriteContextTest.cpp b/mlir/unittests/ABI/ABIRewriteContextTest.cpp index e76c5f3821528..79c9124206562 100644 --- a/mlir/unittests/ABI/ABIRewriteContextTest.cpp +++ b/mlir/unittests/ABI/ABIRewriteContextTest.cpp @@ -36,7 +36,7 @@ TEST(ABIRewriteContextTest, MockCanBeConstructedAndDestroyed) { } TEST(ABIRewriteContextTest, ArgClassificationDirect) { - auto c = ArgClassification::getDirect(/*offset=*/0); + auto c = ArgClassification::getDirect(); EXPECT_EQ(c.kind, ArgKind::Direct); EXPECT_EQ(c.coercedType, nullptr); EXPECT_TRUE(c.canFlatten); @@ -84,8 +84,8 @@ TEST(ABIRewriteContextTest, ArgClassificationExtend) { TEST(ABIRewriteContextTest, FunctionClassificationHoldsReturnAndArgs) { FunctionClassification fc; - fc.returnInfo = ArgClassification::getDirect(/*offset=*/0); - fc.argInfos.push_back(ArgClassification::getDirect(/*offset=*/0)); + fc.returnInfo = ArgClassification::getDirect(); + fc.argInfos.push_back(ArgClassification::getDirect()); fc.argInfos.push_back(ArgClassification::getIndirect(llvm::Align(8), true)); fc.argInfos.push_back(ArgClassification::getIgnore()); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
