[Mlir-commits] [mlir] [MLIR][LLVMIR] Import calls with mismatching signature as indirect call (PR #135895)
Bruno Cardoso Lopes
llvmlistbot at llvm.org
Thu May 1 17:52:23 PDT 2025
https://github.com/bcardosolopes updated https://github.com/llvm/llvm-project/pull/135895
>From b72c3e378a6ac1efb3b31bbe3e561c5a9a2ab282 Mon Sep 17 00:00:00 2001
From: Bruno Cardoso Lopes <bruno.cardoso at gmail.com>
Date: Tue, 15 Apr 2025 18:11:33 -0700
Subject: [PATCH 1/3] [MLIR][LLVMIR] Relax mismatching calls
LLVM IR currently [accepts](https://godbolt.org/z/nqnEsW1ja):
```
define void @incompatible_call_and_callee_types() {
call void @callee(i64 0)
ret void
}
define void @callee({ptr, i64}, i32) {
ret void
}
```
This currently fails to import. Even though these constructs are dangerous and
probably indicate some ODR violation (or optimization bug), they are "valid"
and should be imported into LLVM IR dialect. This PR implements that by using
an indirect call to represent it. Translation already works nicely and outputs
the same source llvm IR file.
The error is now a warning, the tests in
`mlir/test/Target/LLVMIR/Import/import-failure.ll` already use `CHECK` lines,
so no need to add extra diagnostic tests.
---
.../include/mlir/Target/LLVMIR/ModuleImport.h | 9 ++-
mlir/lib/Target/LLVMIR/ModuleImport.cpp | 62 ++++++++++++++-----
.../test/Target/LLVMIR/Import/instructions.ll | 16 +++++
3 files changed, 68 insertions(+), 19 deletions(-)
diff --git a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
index 3dc848c413905..7b01a96026413 100644
--- a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
+++ b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
@@ -362,9 +362,12 @@ class ModuleImport {
/// Converts the callee's function type. For direct calls, it converts the
/// actual function type, which may differ from the called operand type in
/// variadic functions. For indirect calls, it converts the function type
- /// associated with the call instruction. Returns failure when the call and
- /// the callee are not compatible or when nested type conversions failed.
- FailureOr<LLVMFunctionType> convertFunctionType(llvm::CallBase *callInst);
+ /// associated with the call instruction. When the call and the callee are not
+ /// compatible (or when nested type conversions failed), emit a warning but
+ /// attempt translation using a bitcast and an indirect call (in order
+ /// represent valid and verified LLVM IR).
+ FailureOr<LLVMFunctionType> convertFunctionType(llvm::CallBase *callInst,
+ Value &castResult);
/// Returns the callee name, or an empty symbol if the call is not direct.
FlatSymbolRefAttr convertCalleeName(llvm::CallBase *callInst);
/// Converts the parameter and result attributes attached to `func` and adds
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index 0b77a3d23d392..236a5f6a00ec2 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -1721,8 +1721,8 @@ ModuleImport::convertCallOperands(llvm::CallBase *callInst,
/// Checks if `callType` and `calleeType` are compatible and can be represented
/// in MLIR.
static LogicalResult
-verifyFunctionTypeCompatibility(LLVMFunctionType callType,
- LLVMFunctionType calleeType) {
+checkFunctionTypeCompatibility(LLVMFunctionType callType,
+ LLVMFunctionType calleeType) {
if (callType.getReturnType() != calleeType.getReturnType())
return failure();
@@ -1748,7 +1748,7 @@ verifyFunctionTypeCompatibility(LLVMFunctionType callType,
}
FailureOr<LLVMFunctionType>
-ModuleImport::convertFunctionType(llvm::CallBase *callInst) {
+ModuleImport::convertFunctionType(llvm::CallBase *callInst, Value &castResult) {
auto castOrFailure = [](Type convertedType) -> FailureOr<LLVMFunctionType> {
auto funcTy = dyn_cast_or_null<LLVMFunctionType>(convertedType);
if (!funcTy)
@@ -1771,11 +1771,17 @@ ModuleImport::convertFunctionType(llvm::CallBase *callInst) {
if (failed(calleeType))
return failure();
- // Compare the types to avoid constructing illegal call/invoke operations.
- if (failed(verifyFunctionTypeCompatibility(*callType, *calleeType))) {
+ // Compare the types, if they are not compatible, avoid illegal call/invoke
+ // operations by casting to the callsite type and issuing an indirect call.
+ // LLVM IR currently supports this usage.
+ if (failed(checkFunctionTypeCompatibility(*callType, *calleeType))) {
Location loc = translateLoc(callInst->getDebugLoc());
- return emitError(loc) << "incompatible call and callee types: " << *callType
- << " and " << *calleeType;
+ FlatSymbolRefAttr calleeSym = convertCalleeName(callInst);
+ castResult = builder.create<LLVM::AddressOfOp>(
+ loc, LLVM::LLVMPointerType::get(context), calleeSym);
+ emitWarning(loc) << "incompatible call and callee types: " << *callType
+ << " and " << *calleeType;
+ return callType;
}
return calleeType;
@@ -1892,16 +1898,29 @@ LogicalResult ModuleImport::convertInstruction(llvm::Instruction *inst) {
/*operand_attrs=*/nullptr)
.getOperation();
}
- FailureOr<LLVMFunctionType> funcTy = convertFunctionType(callInst);
+ Value castResult;
+ FailureOr<LLVMFunctionType> funcTy =
+ convertFunctionType(callInst, castResult);
if (failed(funcTy))
return failure();
- FlatSymbolRefAttr callee = convertCalleeName(callInst);
- auto callOp = builder.create<CallOp>(loc, *funcTy, callee, *operands);
+ FlatSymbolRefAttr callee = nullptr;
+ // If no cast is needed, use the original callee name. Otherwise patch
+ // operands to include the indirect call target. Build indirect call by
+ // passing using a nullptr `callee`.
+ if (!castResult)
+ callee = convertCalleeName(callInst);
+ else
+ operands->insert(operands->begin(), castResult);
+ CallOp callOp = builder.create<CallOp>(loc, *funcTy, callee, *operands);
+
if (failed(convertCallAttributes(callInst, callOp)))
return failure();
- // Handle parameter and result attributes.
- convertParameterAttributes(callInst, callOp, builder);
+
+ // Handle parameter and result attributes. Don't bother if there's a
+ // type mismatch.
+ if (!castResult)
+ convertParameterAttributes(callInst, callOp, builder);
return callOp.getOperation();
}();
@@ -1966,11 +1985,20 @@ LogicalResult ModuleImport::convertInstruction(llvm::Instruction *inst) {
unwindArgs)))
return failure();
- FailureOr<LLVMFunctionType> funcTy = convertFunctionType(invokeInst);
+ Value castResult;
+ FailureOr<LLVMFunctionType> funcTy =
+ convertFunctionType(invokeInst, castResult);
if (failed(funcTy))
return failure();
- FlatSymbolRefAttr calleeName = convertCalleeName(invokeInst);
+ FlatSymbolRefAttr calleeName = nullptr;
+ // If no cast is needed, use the original callee name. Otherwise patch
+ // operands to include the indirect call target. Build indirect call by
+ // passing using a nullptr `callee`.
+ if (!castResult)
+ calleeName = convertCalleeName(invokeInst);
+ else
+ operands->insert(operands->begin(), castResult);
// Create the invoke operation. Normal destination block arguments will be
// added later on to handle the case in which the operation result is
@@ -1982,8 +2010,10 @@ LogicalResult ModuleImport::convertInstruction(llvm::Instruction *inst) {
if (failed(convertInvokeAttributes(invokeInst, invokeOp)))
return failure();
- // Handle parameter and result attributes.
- convertParameterAttributes(invokeInst, invokeOp, builder);
+ // Handle parameter and result attributes. Don't bother if there's a
+ // type mismatch.
+ if (!castResult)
+ convertParameterAttributes(invokeInst, invokeOp, builder);
if (!invokeInst->getType()->isVoidTy())
mapValue(inst, invokeOp.getResults().front());
diff --git a/mlir/test/Target/LLVMIR/Import/instructions.ll b/mlir/test/Target/LLVMIR/Import/instructions.ll
index 2098d85c18c3f..3e1ce3794abbc 100644
--- a/mlir/test/Target/LLVMIR/Import/instructions.ll
+++ b/mlir/test/Target/LLVMIR/Import/instructions.ll
@@ -739,3 +739,19 @@ bb2:
declare void @g(...)
declare i32 @__gxx_personality_v0(...)
+
+; // -----
+
+; CHECK-LABEL: llvm.func @incompatible_call_and_callee_types
+define void @incompatible_call_and_callee_types() {
+ ; CHECK: %[[CST:.*]] = llvm.mlir.constant(0 : i64) : i64
+ ; CHECK: %[[TARGET:.*]] = llvm.mlir.addressof @callee : !llvm.ptr
+ ; CHECK: llvm.call %[[TARGET]](%[[CST]]) : !llvm.ptr, (i64) -> ()
+ call void @callee(i64 0)
+ ; CHECK: llvm.return
+ ret void
+}
+
+define void @callee({ptr, i64}, i32) {
+ ret void
+}
>From 6fce89c7c10aa9e4e0ce309f9b14acfdf36eb08d Mon Sep 17 00:00:00 2001
From: Bruno Cardoso Lopes <bruno.cardoso at gmail.com>
Date: Thu, 1 May 2025 17:47:59 -0700
Subject: [PATCH 2/3] address review
---
.../include/mlir/Target/LLVMIR/ModuleImport.h | 7 ++--
mlir/lib/Target/LLVMIR/ModuleImport.cpp | 38 ++++++++++---------
2 files changed, 24 insertions(+), 21 deletions(-)
diff --git a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
index 7b01a96026413..b6dffbf5ffa51 100644
--- a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
+++ b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
@@ -364,10 +364,11 @@ class ModuleImport {
/// variadic functions. For indirect calls, it converts the function type
/// associated with the call instruction. When the call and the callee are not
/// compatible (or when nested type conversions failed), emit a warning but
- /// attempt translation using a bitcast and an indirect call (in order
- /// represent valid and verified LLVM IR).
+ /// attempt translation using an indirect call (in order to represent valid
+ /// and verified LLVM IR). The `indirectCallVal` is updated to hold the
+ /// address for the indirect call.
FailureOr<LLVMFunctionType> convertFunctionType(llvm::CallBase *callInst,
- Value &castResult);
+ Value &indirectCallVal);
/// Returns the callee name, or an empty symbol if the call is not direct.
FlatSymbolRefAttr convertCalleeName(llvm::CallBase *callInst);
/// Converts the parameter and result attributes attached to `func` and adds
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index 236a5f6a00ec2..8f856d918a3b3 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -1748,7 +1748,9 @@ checkFunctionTypeCompatibility(LLVMFunctionType callType,
}
FailureOr<LLVMFunctionType>
-ModuleImport::convertFunctionType(llvm::CallBase *callInst, Value &castResult) {
+ModuleImport::convertFunctionType(llvm::CallBase *callInst,
+ Value &indirectCallVal) {
+ indirectCallVal = nullptr;
auto castOrFailure = [](Type convertedType) -> FailureOr<LLVMFunctionType> {
auto funcTy = dyn_cast_or_null<LLVMFunctionType>(convertedType);
if (!funcTy)
@@ -1772,12 +1774,12 @@ ModuleImport::convertFunctionType(llvm::CallBase *callInst, Value &castResult) {
return failure();
// Compare the types, if they are not compatible, avoid illegal call/invoke
- // operations by casting to the callsite type and issuing an indirect call.
- // LLVM IR currently supports this usage.
+ // operations by issuing an indirect call. Note that LLVM IR currently
+ // supports this usage.
if (failed(checkFunctionTypeCompatibility(*callType, *calleeType))) {
Location loc = translateLoc(callInst->getDebugLoc());
FlatSymbolRefAttr calleeSym = convertCalleeName(callInst);
- castResult = builder.create<LLVM::AddressOfOp>(
+ indirectCallVal = builder.create<LLVM::AddressOfOp>(
loc, LLVM::LLVMPointerType::get(context), calleeSym);
emitWarning(loc) << "incompatible call and callee types: " << *callType
<< " and " << *calleeType;
@@ -1898,20 +1900,20 @@ LogicalResult ModuleImport::convertInstruction(llvm::Instruction *inst) {
/*operand_attrs=*/nullptr)
.getOperation();
}
- Value castResult;
+ Value indirectCallVal;
FailureOr<LLVMFunctionType> funcTy =
- convertFunctionType(callInst, castResult);
+ convertFunctionType(callInst, indirectCallVal);
if (failed(funcTy))
return failure();
FlatSymbolRefAttr callee = nullptr;
- // If no cast is needed, use the original callee name. Otherwise patch
- // operands to include the indirect call target. Build indirect call by
- // passing using a nullptr `callee`.
- if (!castResult)
- callee = convertCalleeName(callInst);
+ // If `indirectCallVal` is available emit an indirect call, otherwise use
+ // the callee name. Build an indirect call by passing an empty `callee`
+ // operand and insert into `operands` to include the indirect call target
+ if (indirectCallVal)
+ operands->insert(operands->begin(), indirectCallVal);
else
- operands->insert(operands->begin(), castResult);
+ callee = convertCalleeName(callInst);
CallOp callOp = builder.create<CallOp>(loc, *funcTy, callee, *operands);
if (failed(convertCallAttributes(callInst, callOp)))
@@ -1919,7 +1921,7 @@ LogicalResult ModuleImport::convertInstruction(llvm::Instruction *inst) {
// Handle parameter and result attributes. Don't bother if there's a
// type mismatch.
- if (!castResult)
+ if (!indirectCallVal)
convertParameterAttributes(callInst, callOp, builder);
return callOp.getOperation();
}();
@@ -1985,9 +1987,9 @@ LogicalResult ModuleImport::convertInstruction(llvm::Instruction *inst) {
unwindArgs)))
return failure();
- Value castResult;
+ Value indirectCallVal;
FailureOr<LLVMFunctionType> funcTy =
- convertFunctionType(invokeInst, castResult);
+ convertFunctionType(invokeInst, indirectCallVal);
if (failed(funcTy))
return failure();
@@ -1995,10 +1997,10 @@ LogicalResult ModuleImport::convertInstruction(llvm::Instruction *inst) {
// If no cast is needed, use the original callee name. Otherwise patch
// operands to include the indirect call target. Build indirect call by
// passing using a nullptr `callee`.
- if (!castResult)
+ if (!indirectCallVal)
calleeName = convertCalleeName(invokeInst);
else
- operands->insert(operands->begin(), castResult);
+ operands->insert(operands->begin(), indirectCallVal);
// Create the invoke operation. Normal destination block arguments will be
// added later on to handle the case in which the operation result is
@@ -2012,7 +2014,7 @@ LogicalResult ModuleImport::convertInstruction(llvm::Instruction *inst) {
// Handle parameter and result attributes. Don't bother if there's a
// type mismatch.
- if (!castResult)
+ if (!indirectCallVal)
convertParameterAttributes(invokeInst, invokeOp, builder);
if (!invokeInst->getType()->isVoidTy())
>From 2870639215fd92ac8ba36e8f467aabc2e384d241 Mon Sep 17 00:00:00 2001
From: Bruno Cardoso Lopes <bruno.cardoso at gmail.com>
Date: Thu, 1 May 2025 17:52:08 -0700
Subject: [PATCH 3/3] update one more comment
---
mlir/lib/Target/LLVMIR/ModuleImport.cpp | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index 8f856d918a3b3..854fdadf8cf26 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -1909,7 +1909,7 @@ LogicalResult ModuleImport::convertInstruction(llvm::Instruction *inst) {
FlatSymbolRefAttr callee = nullptr;
// If `indirectCallVal` is available emit an indirect call, otherwise use
// the callee name. Build an indirect call by passing an empty `callee`
- // operand and insert into `operands` to include the indirect call target
+ // operand and insert into `operands` to include the indirect call target.
if (indirectCallVal)
operands->insert(operands->begin(), indirectCallVal);
else
@@ -1919,8 +1919,7 @@ LogicalResult ModuleImport::convertInstruction(llvm::Instruction *inst) {
if (failed(convertCallAttributes(callInst, callOp)))
return failure();
- // Handle parameter and result attributes. Don't bother if there's a
- // type mismatch.
+ // Handle parameter and result attributes unless it's an indirect call.
if (!indirectCallVal)
convertParameterAttributes(callInst, callOp, builder);
return callOp.getOperation();
@@ -1994,9 +1993,9 @@ LogicalResult ModuleImport::convertInstruction(llvm::Instruction *inst) {
return failure();
FlatSymbolRefAttr calleeName = nullptr;
- // If no cast is needed, use the original callee name. Otherwise patch
- // operands to include the indirect call target. Build indirect call by
- // passing using a nullptr `callee`.
+ // If `indirectCallVal` is available emit an indirect call, otherwise use
+ // the callee name. Build an indirect call by passing an empty `callee`
+ // operand and insert into `operands` to include the indirect call target.
if (!indirectCallVal)
calleeName = convertCalleeName(invokeInst);
else
@@ -2012,8 +2011,7 @@ LogicalResult ModuleImport::convertInstruction(llvm::Instruction *inst) {
if (failed(convertInvokeAttributes(invokeInst, invokeOp)))
return failure();
- // Handle parameter and result attributes. Don't bother if there's a
- // type mismatch.
+ // Handle parameter and result attributes unless it's an indirect call.
if (!indirectCallVal)
convertParameterAttributes(invokeInst, invokeOp, builder);
More information about the Mlir-commits
mailing list