https://github.com/adams381 updated https://github.com/llvm/llvm-project/pull/214983
>From b1c80f41b6c694886d4996d2d8c6d2c8ccb2ccfb Mon Sep 17 00:00:00 2001 From: Adam Smith <[email protected]> Date: Sat, 8 Aug 2026 09:02:29 -0700 Subject: [PATCH 1/2] [CIR] Fix two x86_64 signatures that lowered silently wrong Fix two x86_64 signatures that lower silently wrong code for hand-written CIR. x86_64 calling-convention lowering: 1) drops an array argument of three to eight eightbytes instead of passing it in memory. 2) lowers a callee's signature while leaving an indirect cir.try_call to that callee in the form it was written. The pass cannot assume a classification is safe to act on, so an Ignore that would drop a value still carrying data is now an error, for the return as well as an argument. The try_call reaches the call-site rewriter, which reports it because it cannot rebuild a cir.try_call yet. Assisted-by: Cursor / claude-opus-5 --- .../Transforms/CallConvLoweringPass.cpp | 47 +++++++++---- .../indirect-call-classification-attr.cir | 45 ++++++++++++- .../abi-lowering/x86_64-array-drop-nyi.cir | 66 +++++++++++++++++++ .../abi-lowering/x86_64-empty-record.cir | 19 ++++++ .../x86_64-indirect-try-call-nyi.cir | 39 +++++++++++ .../abi-lowering/x86_64-struct-direct.cir | 10 +++ 6 files changed, 212 insertions(+), 14 deletions(-) create mode 100644 clang/test/CIR/Transforms/abi-lowering/x86_64-array-drop-nyi.cir create mode 100644 clang/test/CIR/Transforms/abi-lowering/x86_64-indirect-try-call-nyi.cir diff --git a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp index 193c2b6f4a9dc..13752edabec48 100644 --- a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp +++ b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp @@ -356,6 +356,25 @@ convertABIArgInfo(const llvm::abi::ArgInfo &info, MLIRContext *ctx, return ArgClassification::getIgnore(); } +/// Whether dropping \p ty from the signature would lose data. Ignore is the +/// ABI's answer for a void type and for an aggregate holding no data, and +/// dropping one of those is correct. A classification that would drop +/// anything else has to be reported instead, because nothing would be passed +/// in its place. +/// +/// An aggregate with no members counts as holding no data. That relies on +/// CIRGen only omitting a member that is itself empty, which is what makes an +/// over-aligned union of empty classes droppable here the way the ABI wants. +static bool ignoreLosesData(mlir::Type ty) { + if (!ty || isa<cir::VoidType>(ty)) + return false; + if (auto recTy = dyn_cast<cir::RecordType>(ty)) + return llvm::any_of(recTy.getMembers(), ignoreLosesData); + if (auto arrTy = dyn_cast<cir::ArrayType>(ty)) + return arrTy.getSize() != 0 && ignoreLosesData(arrTy.getElementType()); + return true; +} + /// Where \p fnTy's declared parameters end and its ellipsis arguments begin. /// /// The only x86_64 rule that reads this boundary sends an unnamed vector wider @@ -421,6 +440,17 @@ static std::optional<FunctionClassification> classifyX86_64Signature( << t; }; + // An Ignore the ABI does not mean, where rewriting the signature would pass + // nothing in the value's place. + auto nyiDrop = [&](const ArgClassification &ac, mlir::Type t, + llvm::StringRef what) { + if (ac.kind != ArgKind::Ignore || !ignoreLosesData(t)) + return false; + emitError() << "x86_64 calling-convention lowering would drop " << what + << " of type " << t << ", which is not yet implemented"; + return true; + }; + FunctionClassification fc; fc.returnsVoid = voidRet; mlir::Type origRet = voidRet ? mlir::Type() : retCIR; @@ -430,6 +460,8 @@ static std::optional<FunctionClassification> classifyX86_64Signature( nyiCoercion(retCIR); return std::nullopt; } + if (nyiDrop(*retAc, origRet, "the return value")) + return std::nullopt; fc.returnInfo = *retAc; for (unsigned i = 0, e = fi->arg_size(); i < e; ++i) { mlir::Type origArg = i < inputs.size() ? inputs[i] : mlir::Type(); @@ -439,6 +471,8 @@ static std::optional<FunctionClassification> classifyX86_64Signature( nyiCoercion(origArg); return std::nullopt; } + if (nyiDrop(*ac, origArg, "an argument")) + return std::nullopt; fc.argInfos.push_back(*ac); } return fc; @@ -750,17 +784,8 @@ void CallConvLoweringPass::runOnOperation() { // cached as the next one to visit. SmallVector<cir::CIRCallOpInterface> indirectCalls; moduleOp.walk([&](cir::CIRCallOpInterface c) { - cir::FuncType calleeTy = indirectCalleeType(c); - if (!calleeTy) - return; - // A cir.try_call is in this walk so that a variadic one reaches the - // ellipsis accounting below. CIRABIRewriteContext cannot rebuild a - // cir.try_call at all, so a non-variadic one has never been rewritten - // here. Keep it out rather than start reporting a gap that has nothing - // to do with the ellipsis. - if (!calleeTy.isVarArg() && isa<cir::TryCallOp>(c.getOperation())) - return; - indirectCalls.push_back(c); + if (indirectCalleeType(c)) + indirectCalls.push_back(c); }); for (cir::CIRCallOpInterface c : indirectCalls) { // classification-attr mode injects a per-function classification, which diff --git a/clang/test/CIR/Transforms/abi-lowering/indirect-call-classification-attr.cir b/clang/test/CIR/Transforms/abi-lowering/indirect-call-classification-attr.cir index 9c709d4ee8042..f9732688dcd03 100644 --- a/clang/test/CIR/Transforms/abi-lowering/indirect-call-classification-attr.cir +++ b/clang/test/CIR/Transforms/abi-lowering/indirect-call-classification-attr.cir @@ -42,9 +42,9 @@ module attributes { dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<i32, dense<32>: vector<2xi64>>> } { - // A variadic indirect cir.try_call reaches the same driver-mode error. It is - // collected so that its ellipsis is accounted for, which a per-function - // injected classification cannot describe either. + // An indirect cir.try_call reaches the same driver-mode error, whether or + // not its callee is variadic, since a per-function injected classification + // cannot describe a callee resolved at run time either way. cir.func @variadic_try_caller(%fp: !cir.ptr<!cir.func<(!cir.ptr<!s8i>, ...) -> !s32i>>, %fmt: !cir.ptr<!s8i>, %n: !s32i) -> !s32i attributes { test_classify = #passthrough } { @@ -69,3 +69,42 @@ module attributes { } // CHECK: error: 'cir.try_call' op indirect call cannot be classified in the 'classification-attr' driver mode + +// ----- + +!s8i = !cir.int<s, 8> +!s32i = !cir.int<s, 32> +!void = !cir.void + +#passthrough = { + return = { kind = "direct" }, + args = [ { kind = "direct" }, { kind = "direct" } ] +} + +module attributes { + dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<i32, dense<32>: vector<2xi64>>> +} { + + cir.func @try_caller(%fp: !cir.ptr<!cir.func<(!s32i) -> !s32i>>, %n: !s32i) + -> !s32i attributes { test_classify = #passthrough } { + %0 = cir.try_call %fp(%n) ^bb1, ^bb2 + : (!cir.ptr<!cir.func<(!s32i) -> !s32i>>, !s32i) -> !s32i + ^bb1: + cir.br ^bb4(%0 : !s32i) + ^bb2: + %1 = cir.eh.initiate : !cir.eh_token + cir.eh.dispatch %1 : !cir.eh_token [ + catch_all : ^bb3 + ] + ^bb3(%tok : !cir.eh_token): + %ct, %exn = cir.begin_catch %tok : !cir.eh_token -> (!cir.catch_token, !cir.ptr<!void>) + cir.end_catch %ct : !cir.catch_token + %2 = cir.const #cir.int<0> : !s32i + cir.br ^bb4(%2 : !s32i) + ^bb4(%r : !s32i): + cir.return %r : !s32i + } + +} + +// CHECK: error: 'cir.try_call' op indirect call cannot be classified in the 'classification-attr' driver mode diff --git a/clang/test/CIR/Transforms/abi-lowering/x86_64-array-drop-nyi.cir b/clang/test/CIR/Transforms/abi-lowering/x86_64-array-drop-nyi.cir new file mode 100644 index 0000000000000..fdcf5ce4d5a2f --- /dev/null +++ b/clang/test/CIR/Transforms/abi-lowering/x86_64-array-drop-nyi.cir @@ -0,0 +1,66 @@ +// RUN: not cir-opt %s -cir-call-conv-lowering=target=x86_64 \ +// RUN: -split-input-file 2>&1 | FileCheck %s + +!s64i = !cir.int<s, 64> + +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>>> +} { + + // The classifier answers Ignore for an array spanning three to eight + // eightbytes, where SysV says memory. Rewriting that answer would pass + // nothing in the value's place, so it is reported instead. Two eightbytes + // in registers and nine byval are both in x86_64-struct-direct.cir. + cir.func @take_arr_3eb(%arg0: !cir.array<!s64i x 3>) { + cir.return + } + + // CHECK: error: 'cir.func' op x86_64 calling-convention lowering would drop an argument of type '!cir.array<!cir.int<s, 64> x 3>', which is not yet implemented + + // Eight eightbytes is the last size the classifier answers Ignore for. + cir.func @take_arr_8eb(%arg0: !cir.array<!s64i x 8>) { + cir.return + } + + // CHECK: error: 'cir.func' op x86_64 calling-convention lowering would drop an argument of type '!cir.array<!cir.int<s, 64> x 8>', which is not yet implemented + + // The return value goes through the same guard and reports its own position. + cir.func @ret_arr_3eb() -> !cir.array<!s64i x 3> { + %0 = cir.alloca "r" align(8) : !cir.ptr<!cir.array<!s64i x 3>> + %1 = cir.load %0 : !cir.ptr<!cir.array<!s64i x 3>>, !cir.array<!s64i x 3> + cir.return %1 : !cir.array<!s64i x 3> + } + + // CHECK: error: 'cir.func' op x86_64 calling-convention lowering would drop the return value of type '!cir.array<!cir.int<s, 64> x 3>', which is not yet implemented +} + +// ----- + +// An indirect callee carries the array only in its pointee signature, so the +// report is anchored on the call rather than on the enclosing function. It +// needs its own module because a signature failure above stops the pass before +// the indirect-call walk. + +!s64i = !cir.int<s, 64> + +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>>> +} { + + cir.func @call_arr_3eb(%arg0: !cir.ptr<!cir.func<(!cir.array<!s64i x 3>)>>) { + %0 = cir.alloca "a" align(8) : !cir.ptr<!cir.array<!s64i x 3>> + %1 = cir.load %0 : !cir.ptr<!cir.array<!s64i x 3>>, !cir.array<!s64i x 3> + cir.call %arg0(%1) : (!cir.ptr<!cir.func<(!cir.array<!s64i x 3>)>>, !cir.array<!s64i x 3>) -> () + cir.return + } + + // CHECK: error: 'cir.call' op x86_64 calling-convention lowering would drop an argument of type '!cir.array<!cir.int<s, 64> x 3>', which is not yet implemented +} diff --git a/clang/test/CIR/Transforms/abi-lowering/x86_64-empty-record.cir b/clang/test/CIR/Transforms/abi-lowering/x86_64-empty-record.cir index 23254734f27e5..5dc3db0509ffd 100644 --- a/clang/test/CIR/Transforms/abi-lowering/x86_64-empty-record.cir +++ b/clang/test/CIR/Transforms/abi-lowering/x86_64-empty-record.cir @@ -61,4 +61,23 @@ module attributes { // CHECK: cir.func{{.*}} @caller_local(%arg0: !s32i) -> !s32i // CHECK: cir.alloca "e" align(1) : !cir.ptr<!rec_E0> // CHECK: cir.call @take_mixed(%arg0) : (!s32i) -> !s32i + + // A zero-length array holds no data, so Ignore is the ABI's answer and the + // argument is dropped rather than reported. A longer array of the same + // element type is reported instead, in x86_64-array-drop-nyi.cir. + cir.func @take_arr_empty(%arg0: !cir.array<!s32i x 0>) { + cir.return + } + + // CHECK: cir.func{{.*}} @take_arr_empty() + // CHECK-NEXT: cir.return + + // The same holds through a record, which is what the member walk answers + // for a wrapper whose fields are all empty. + cir.func @take_wrapped_empty(%arg0: !cir.struct<"WE" {!cir.array<!s32i x 0>, !rec_E0}>) { + cir.return + } + + // CHECK: cir.func{{.*}} @take_wrapped_empty() + // CHECK-NEXT: cir.return } diff --git a/clang/test/CIR/Transforms/abi-lowering/x86_64-indirect-try-call-nyi.cir b/clang/test/CIR/Transforms/abi-lowering/x86_64-indirect-try-call-nyi.cir new file mode 100644 index 0000000000000..c2866f0d2e869 --- /dev/null +++ b/clang/test/CIR/Transforms/abi-lowering/x86_64-indirect-try-call-nyi.cir @@ -0,0 +1,39 @@ +// RUN: not cir-opt %s -cir-call-conv-lowering=target=x86_64 2>&1 | FileCheck %s + +!s8i = !cir.int<s, 8> +!s32i = !cir.int<s, 32> +!void = !cir.void + +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>>> +} { + + // A narrow integer argument needs signext, so unlike @pass_through in + // x86_64-indirect-try-call.cir this call does not already carry its wire + // form. CIRABIRewriteContext cannot rebuild a cir.try_call, so it is + // reported. + cir.func @needs_rewrite(%arg0: !cir.ptr<!cir.func<(!s8i) -> !s32i>>, + %arg1: !s8i) { + %0 = cir.try_call %arg0(%arg1) ^bb1, ^bb2 + : (!cir.ptr<!cir.func<(!s8i) -> !s32i>>, !s8i) -> !s32i + ^bb1: + cir.br ^bb4 + ^bb2: + %1 = cir.eh.initiate : !cir.eh_token + cir.eh.dispatch %1 : !cir.eh_token [ + catch_all : ^bb3 + ] + ^bb3(%tok : !cir.eh_token): + %ct, %exn = cir.begin_catch %tok : !cir.eh_token -> (!cir.catch_token, !cir.ptr<!void>) + cir.end_catch %ct : !cir.catch_token + cir.br ^bb4 + ^bb4: + cir.return + } +} + +// CHECK: error: 'cir.try_call' op TryCallOp not yet implemented in CallConvLowering diff --git a/clang/test/CIR/Transforms/abi-lowering/x86_64-struct-direct.cir b/clang/test/CIR/Transforms/abi-lowering/x86_64-struct-direct.cir index 22cd3c68f2149..b46605ecea0c1 100644 --- a/clang/test/CIR/Transforms/abi-lowering/x86_64-struct-direct.cir +++ b/clang/test/CIR/Transforms/abi-lowering/x86_64-struct-direct.cir @@ -118,6 +118,15 @@ module attributes { // CHECK: cir.store %arg1, %[[E1]] : !s32i, !cir.ptr<!s32i> // CHECK: %{{.*}} = cir.cast bitcast %{{.*}} : !cir.ptr<!rec_anon_struct1> -> !cir.ptr<!cir.array<!s32i x 3>> + // Above eight eightbytes an array argument goes to memory. Seventy-two + // bytes here and twelve bytes in take_arr3 above are the sizes either side + // of the window x86_64-array-drop-nyi.cir reports. + cir.func @take_arr9(%arg0: !cir.array<!s64i x 9>) { + cir.return + } + + // CHECK: cir.func{{.*}} @take_arr9(%arg0: !cir.ptr<!cir.array<!s64i x 9>> {llvm.align = 8 : i64, llvm.byval = !cir.array<!s64i x 9>, llvm.noalias, llvm.noundef}) + // An anonymous struct has no record-layout entry, so it defaults to // can-pass-in-registers and coerces like its named counterpart. cir.func @take_anon(%arg0: !cir.struct<{!s32i, !s32i}>) { @@ -168,6 +177,7 @@ module attributes { // LLVM: define void @take_charbuf(i32 %{{.+}}) // LLVM: define void @take_char1(i8 %{{.+}}) // LLVM: define void @take_arr3(i64 %{{.+}}, i32 %{{.+}}) +// LLVM: define void @take_arr9(ptr noalias noundef byval([9 x i64]) align 8 %{{[^,)]+}}) // LLVM: define void @take_anon(i64 %{{.+}}) // LLVM: define void @call_pair(i64 %{{.+}}) // LLVM: %{{.+}} = call i32 @take_pair(i64 %{{.+}}) >From 99690e25e2fcb4b9cab1134a2bca17cf69bf1de4 Mon Sep 17 00:00:00 2001 From: Adam Smith <[email protected]> Date: Mon, 10 Aug 2026 19:06:08 -0700 Subject: [PATCH 2/2] [CIR][NFC] Reword the comment on the callconv Ignore guard The comment above nyiDrop read "An Ignore the ABI does not mean, where rewriting the signature would pass nothing in the value's place." This doesn't tell the whole story and might confuse a reader. Spell out the distinction instead: Ignore is correct for a void type or an aggregate holding no data, the classifier also reaches it for a value that still carries data, and rewriting on that second answer would leave the signature with nothing in the value's place, so it is reported as NYI. Assisted-by: Cursor / claude-opus-5 --- clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp index 13752edabec48..abfc34501d3d5 100644 --- a/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp +++ b/clang/lib/CIR/Dialect/Transforms/CallConvLoweringPass.cpp @@ -440,8 +440,10 @@ static std::optional<FunctionClassification> classifyX86_64Signature( << t; }; - // An Ignore the ABI does not mean, where rewriting the signature would pass - // nothing in the value's place. + // Ignore is the ABI's answer for a void type or an aggregate holding no + // data, but the classifier also reaches it for a value that still carries + // data. Acting on that answer would leave the rewritten signature with + // nothing in the value's place, so report it as NYI instead. auto nyiDrop = [&](const ArgClassification &ac, mlir::Type t, llvm::StringRef what) { if (ac.kind != ArgKind::Ignore || !ignoreLosesData(t)) _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
