Revert "[MLIR][LLVMIR] Import unregistered intrinsics via llvm.intrinsic_call" #128973
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…sic_call…" This reverts commit d584d1f.
|
@llvm/pr-subscribers-mlir Author: Bruno Cardoso Lopes (bcardosolopes) ChangesReverts llvm/llvm-project#128626 Looks like the static definition broke some bots! Full diff: https://github.com/llvm/llvm-project/pull/128973.diff 4 Files Affected:
diff --git a/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h b/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h
index 686969f891f20..cc5a77ed35d2b 100644
--- a/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h
+++ b/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h
@@ -155,18 +155,9 @@ class LLVMImportInterface
LogicalResult convertIntrinsic(OpBuilder &builder, llvm::CallInst *inst,
LLVM::ModuleImport &moduleImport) const {
// Lookup the dialect interface for the given intrinsic.
- // Verify the intrinsic identifier maps to an actual intrinsic.
- llvm::Intrinsic::ID intrinId = inst->getIntrinsicID();
- assert(intrinId != llvm::Intrinsic::not_intrinsic);
-
- // First lookup the intrinsic across different dialects for known
- // supported conversions, examples include arm-neon, nvm-sve, etc.
- Dialect *dialect = intrinsicToDialect.lookup(intrinId);
-
- // No specialized (supported) intrinsics, attempt to generate a generic
- // version via llvm.call_intrinsic (if available).
+ Dialect *dialect = intrinsicToDialect.lookup(inst->getIntrinsicID());
if (!dialect)
- return convertUnregisteredIntrinsic(builder, inst, moduleImport);
+ return failure();
// Dispatch the conversion to the dialect interface.
const LLVMImportDialectInterface *iface = getInterfaceFor(dialect);
@@ -233,11 +224,6 @@ class LLVMImportInterface
}
private:
- /// Generate llvm.call_intrinsic when no supporting dialect available.
- static LogicalResult
- convertUnregisteredIntrinsic(OpBuilder &builder, llvm::CallInst *inst,
- LLVM::ModuleImport &moduleImport);
-
DenseMap<unsigned, Dialect *> intrinsicToDialect;
DenseMap<unsigned, const LLVMImportDialectInterface *> instructionToDialect;
DenseMap<unsigned, SmallVector<Dialect *, 1>> metadataToDialect;
diff --git a/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp
index 227779f355069..4fd043c7c93e6 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp
@@ -367,45 +367,6 @@ static LogicalResult setIntelReqdSubGroupSizeAttr(Builder &builder,
return success();
}
-LogicalResult mlir::LLVMImportInterface::convertUnregisteredIntrinsic(
- OpBuilder &builder, llvm::CallInst *inst,
- LLVM::ModuleImport &moduleImport) {
- StringRef intrinName = inst->getCalledFunction()->getName();
-
- SmallVector<llvm::Value *> args(inst->args());
- ArrayRef<llvm::Value *> llvmOperands(args);
-
- SmallVector<llvm::OperandBundleUse> llvmOpBundles;
- llvmOpBundles.reserve(inst->getNumOperandBundles());
- for (unsigned i = 0; i < inst->getNumOperandBundles(); ++i)
- llvmOpBundles.push_back(inst->getOperandBundleAt(i));
-
- SmallVector<Value> mlirOperands;
- SmallVector<NamedAttribute> mlirAttrs;
- if (failed(moduleImport.convertIntrinsicArguments(
- llvmOperands, llvmOpBundles, false, {}, {}, mlirOperands, mlirAttrs)))
- return failure();
-
- Type results = moduleImport.convertType(inst->getType());
- auto op = builder.create<::mlir::LLVM::CallIntrinsicOp>(
- moduleImport.translateLoc(inst->getDebugLoc()), results,
- StringAttr::get(builder.getContext(), intrinName),
- ValueRange{mlirOperands}, FastmathFlagsAttr{});
-
- moduleImport.setFastmathFlagsAttr(inst, op);
-
- // Update importer tracking of results.
- unsigned numRes = op.getNumResults();
- if (numRes == 1)
- moduleImport.mapValue(inst) = op.getResult(0);
- else if (numRes == 0)
- moduleImport.mapNoResultOp(inst);
- else
- return op.emitError(
- "expected at most one result from target intrinsic call");
-
- return success();
-}
namespace {
/// Implementation of the dialect interface that converts operations belonging
diff --git a/mlir/test/Target/LLVMIR/Import/import-failure.ll b/mlir/test/Target/LLVMIR/Import/import-failure.ll
index fc4ccddb756d5..d929a59284762 100644
--- a/mlir/test/Target/LLVMIR/Import/import-failure.ll
+++ b/mlir/test/Target/LLVMIR/Import/import-failure.ll
@@ -38,6 +38,18 @@ bb1:
; // -----
+declare void @llvm.gcroot(ptr %arg1, ptr %arg2)
+
+; CHECK: <unknown>
+; CHECK-SAME: error: unhandled intrinsic: call void @llvm.gcroot(ptr %arg1, ptr null)
+define void @unhandled_intrinsic() gc "example" {
+ %arg1 = alloca ptr
+ call void @llvm.gcroot(ptr %arg1, ptr null)
+ ret void
+}
+
+; // -----
+
; Check that debug intrinsics with an unsupported argument are dropped.
declare void @llvm.dbg.value(metadata, metadata, metadata)
diff --git a/mlir/test/Target/LLVMIR/Import/intrinsic-unregistered.ll b/mlir/test/Target/LLVMIR/Import/intrinsic-unregistered.ll
deleted file mode 100644
index 554be8f797b75..0000000000000
--- a/mlir/test/Target/LLVMIR/Import/intrinsic-unregistered.ll
+++ /dev/null
@@ -1,68 +0,0 @@
-; RUN: mlir-translate -import-llvm %s -split-input-file | FileCheck %s
-
-declare i64 @llvm.aarch64.ldxr.p0(ptr)
-
-define dso_local void @t0(ptr %a) {
- %x = call i64 @llvm.aarch64.ldxr.p0(ptr elementtype(i8) %a)
- ret void
-}
-
-; CHECK-LABEL: llvm.func @llvm.aarch64.ldxr.p0(!llvm.ptr)
-; CHECK-LABEL: llvm.func @t0
-; CHECK: llvm.call_intrinsic "llvm.aarch64.ldxr.p0"({{.*}}) : (!llvm.ptr) -> i64
-; CHECK: llvm.return
-; CHECK: }
-
-; // -----
-
-declare <8 x i8> @llvm.aarch64.neon.uabd.v8i8(<8 x i8>, <8 x i8>)
-
-define dso_local <8 x i8> @t1(<8 x i8> %lhs, <8 x i8> %rhs) {
- %r = call <8 x i8> @llvm.aarch64.neon.uabd.v8i8(<8 x i8> %lhs, <8 x i8> %rhs)
- ret <8 x i8> %r
-}
-
-; CHECK: llvm.func @t1(%[[A0:.*]]: vector<8xi8>, %[[A1:.*]]: vector<8xi8>) -> vector<8xi8> {{.*}} {
-; CHECK: %[[R:.*]] = llvm.call_intrinsic "llvm.aarch64.neon.uabd.v8i8"(%[[A0]], %[[A1]]) : (vector<8xi8>, vector<8xi8>) -> vector<8xi8>
-; CHECK: llvm.return %[[R]] : vector<8xi8>
-; CHECK: }
-
-; // -----
-
-declare void @llvm.aarch64.neon.st2.v8i8.p0(<8 x i8>, <8 x i8>, ptr)
-
-define dso_local void @t2(<8 x i8> %lhs, <8 x i8> %rhs, ptr %a) {
- call void @llvm.aarch64.neon.st2.v8i8.p0(<8 x i8> %lhs, <8 x i8> %rhs, ptr %a)
- ret void
-}
-
-; CHECK: llvm.func @t2(%[[A0:.*]]: vector<8xi8>, %[[A1:.*]]: vector<8xi8>, %[[A2:.*]]: !llvm.ptr) {{.*}} {
-; CHECK: llvm.call_intrinsic "llvm.aarch64.neon.st2.v8i8.p0"(%[[A0]], %[[A1]], %[[A2]]) : (vector<8xi8>, vector<8xi8>, !llvm.ptr) -> !llvm.void
-; CHECK: llvm.return
-; CHECK: }
-
-; // -----
-
-declare void @llvm.gcroot(ptr %arg1, ptr %arg2)
-define void @gctest() gc "example" {
- %arg1 = alloca ptr
- call void @llvm.gcroot(ptr %arg1, ptr null)
- ret void
-}
-
-; CHECK-LABEL: @gctest
-; CHECK: llvm.call_intrinsic "llvm.gcroot"({{.*}}, {{.*}}) : (!llvm.ptr, !llvm.ptr) -> !llvm.void
-
-; // -----
-
-; Test we get the supported version, not the unregistered one.
-
-declare i32 @llvm.lround.i32.f32(float)
-
-; CHECK-LABEL: llvm.func @lround_test
-define void @lround_test(float %0, double %1) {
- ; CHECK-NOT: llvm.call_intrinsic "llvm.lround
- ; CHECK: llvm.intr.lround(%{{.*}}) : (f32) -> i32
- %3 = call i32 @llvm.lround.i32.f32(float %0)
- ret void
-}
|
|
@llvm/pr-subscribers-mlir-llvm Author: Bruno Cardoso Lopes (bcardosolopes) ChangesReverts llvm/llvm-project#128626 Looks like the static definition broke some bots! Full diff: https://github.com/llvm/llvm-project/pull/128973.diff 4 Files Affected:
diff --git a/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h b/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h
index 686969f891f20..cc5a77ed35d2b 100644
--- a/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h
+++ b/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h
@@ -155,18 +155,9 @@ class LLVMImportInterface
LogicalResult convertIntrinsic(OpBuilder &builder, llvm::CallInst *inst,
LLVM::ModuleImport &moduleImport) const {
// Lookup the dialect interface for the given intrinsic.
- // Verify the intrinsic identifier maps to an actual intrinsic.
- llvm::Intrinsic::ID intrinId = inst->getIntrinsicID();
- assert(intrinId != llvm::Intrinsic::not_intrinsic);
-
- // First lookup the intrinsic across different dialects for known
- // supported conversions, examples include arm-neon, nvm-sve, etc.
- Dialect *dialect = intrinsicToDialect.lookup(intrinId);
-
- // No specialized (supported) intrinsics, attempt to generate a generic
- // version via llvm.call_intrinsic (if available).
+ Dialect *dialect = intrinsicToDialect.lookup(inst->getIntrinsicID());
if (!dialect)
- return convertUnregisteredIntrinsic(builder, inst, moduleImport);
+ return failure();
// Dispatch the conversion to the dialect interface.
const LLVMImportDialectInterface *iface = getInterfaceFor(dialect);
@@ -233,11 +224,6 @@ class LLVMImportInterface
}
private:
- /// Generate llvm.call_intrinsic when no supporting dialect available.
- static LogicalResult
- convertUnregisteredIntrinsic(OpBuilder &builder, llvm::CallInst *inst,
- LLVM::ModuleImport &moduleImport);
-
DenseMap<unsigned, Dialect *> intrinsicToDialect;
DenseMap<unsigned, const LLVMImportDialectInterface *> instructionToDialect;
DenseMap<unsigned, SmallVector<Dialect *, 1>> metadataToDialect;
diff --git a/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp
index 227779f355069..4fd043c7c93e6 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp
@@ -367,45 +367,6 @@ static LogicalResult setIntelReqdSubGroupSizeAttr(Builder &builder,
return success();
}
-LogicalResult mlir::LLVMImportInterface::convertUnregisteredIntrinsic(
- OpBuilder &builder, llvm::CallInst *inst,
- LLVM::ModuleImport &moduleImport) {
- StringRef intrinName = inst->getCalledFunction()->getName();
-
- SmallVector<llvm::Value *> args(inst->args());
- ArrayRef<llvm::Value *> llvmOperands(args);
-
- SmallVector<llvm::OperandBundleUse> llvmOpBundles;
- llvmOpBundles.reserve(inst->getNumOperandBundles());
- for (unsigned i = 0; i < inst->getNumOperandBundles(); ++i)
- llvmOpBundles.push_back(inst->getOperandBundleAt(i));
-
- SmallVector<Value> mlirOperands;
- SmallVector<NamedAttribute> mlirAttrs;
- if (failed(moduleImport.convertIntrinsicArguments(
- llvmOperands, llvmOpBundles, false, {}, {}, mlirOperands, mlirAttrs)))
- return failure();
-
- Type results = moduleImport.convertType(inst->getType());
- auto op = builder.create<::mlir::LLVM::CallIntrinsicOp>(
- moduleImport.translateLoc(inst->getDebugLoc()), results,
- StringAttr::get(builder.getContext(), intrinName),
- ValueRange{mlirOperands}, FastmathFlagsAttr{});
-
- moduleImport.setFastmathFlagsAttr(inst, op);
-
- // Update importer tracking of results.
- unsigned numRes = op.getNumResults();
- if (numRes == 1)
- moduleImport.mapValue(inst) = op.getResult(0);
- else if (numRes == 0)
- moduleImport.mapNoResultOp(inst);
- else
- return op.emitError(
- "expected at most one result from target intrinsic call");
-
- return success();
-}
namespace {
/// Implementation of the dialect interface that converts operations belonging
diff --git a/mlir/test/Target/LLVMIR/Import/import-failure.ll b/mlir/test/Target/LLVMIR/Import/import-failure.ll
index fc4ccddb756d5..d929a59284762 100644
--- a/mlir/test/Target/LLVMIR/Import/import-failure.ll
+++ b/mlir/test/Target/LLVMIR/Import/import-failure.ll
@@ -38,6 +38,18 @@ bb1:
; // -----
+declare void @llvm.gcroot(ptr %arg1, ptr %arg2)
+
+; CHECK: <unknown>
+; CHECK-SAME: error: unhandled intrinsic: call void @llvm.gcroot(ptr %arg1, ptr null)
+define void @unhandled_intrinsic() gc "example" {
+ %arg1 = alloca ptr
+ call void @llvm.gcroot(ptr %arg1, ptr null)
+ ret void
+}
+
+; // -----
+
; Check that debug intrinsics with an unsupported argument are dropped.
declare void @llvm.dbg.value(metadata, metadata, metadata)
diff --git a/mlir/test/Target/LLVMIR/Import/intrinsic-unregistered.ll b/mlir/test/Target/LLVMIR/Import/intrinsic-unregistered.ll
deleted file mode 100644
index 554be8f797b75..0000000000000
--- a/mlir/test/Target/LLVMIR/Import/intrinsic-unregistered.ll
+++ /dev/null
@@ -1,68 +0,0 @@
-; RUN: mlir-translate -import-llvm %s -split-input-file | FileCheck %s
-
-declare i64 @llvm.aarch64.ldxr.p0(ptr)
-
-define dso_local void @t0(ptr %a) {
- %x = call i64 @llvm.aarch64.ldxr.p0(ptr elementtype(i8) %a)
- ret void
-}
-
-; CHECK-LABEL: llvm.func @llvm.aarch64.ldxr.p0(!llvm.ptr)
-; CHECK-LABEL: llvm.func @t0
-; CHECK: llvm.call_intrinsic "llvm.aarch64.ldxr.p0"({{.*}}) : (!llvm.ptr) -> i64
-; CHECK: llvm.return
-; CHECK: }
-
-; // -----
-
-declare <8 x i8> @llvm.aarch64.neon.uabd.v8i8(<8 x i8>, <8 x i8>)
-
-define dso_local <8 x i8> @t1(<8 x i8> %lhs, <8 x i8> %rhs) {
- %r = call <8 x i8> @llvm.aarch64.neon.uabd.v8i8(<8 x i8> %lhs, <8 x i8> %rhs)
- ret <8 x i8> %r
-}
-
-; CHECK: llvm.func @t1(%[[A0:.*]]: vector<8xi8>, %[[A1:.*]]: vector<8xi8>) -> vector<8xi8> {{.*}} {
-; CHECK: %[[R:.*]] = llvm.call_intrinsic "llvm.aarch64.neon.uabd.v8i8"(%[[A0]], %[[A1]]) : (vector<8xi8>, vector<8xi8>) -> vector<8xi8>
-; CHECK: llvm.return %[[R]] : vector<8xi8>
-; CHECK: }
-
-; // -----
-
-declare void @llvm.aarch64.neon.st2.v8i8.p0(<8 x i8>, <8 x i8>, ptr)
-
-define dso_local void @t2(<8 x i8> %lhs, <8 x i8> %rhs, ptr %a) {
- call void @llvm.aarch64.neon.st2.v8i8.p0(<8 x i8> %lhs, <8 x i8> %rhs, ptr %a)
- ret void
-}
-
-; CHECK: llvm.func @t2(%[[A0:.*]]: vector<8xi8>, %[[A1:.*]]: vector<8xi8>, %[[A2:.*]]: !llvm.ptr) {{.*}} {
-; CHECK: llvm.call_intrinsic "llvm.aarch64.neon.st2.v8i8.p0"(%[[A0]], %[[A1]], %[[A2]]) : (vector<8xi8>, vector<8xi8>, !llvm.ptr) -> !llvm.void
-; CHECK: llvm.return
-; CHECK: }
-
-; // -----
-
-declare void @llvm.gcroot(ptr %arg1, ptr %arg2)
-define void @gctest() gc "example" {
- %arg1 = alloca ptr
- call void @llvm.gcroot(ptr %arg1, ptr null)
- ret void
-}
-
-; CHECK-LABEL: @gctest
-; CHECK: llvm.call_intrinsic "llvm.gcroot"({{.*}}, {{.*}}) : (!llvm.ptr, !llvm.ptr) -> !llvm.void
-
-; // -----
-
-; Test we get the supported version, not the unregistered one.
-
-declare i32 @llvm.lround.i32.f32(float)
-
-; CHECK-LABEL: llvm.func @lround_test
-define void @lround_test(float %0, double %1) {
- ; CHECK-NOT: llvm.call_intrinsic "llvm.lround
- ; CHECK: llvm.intr.lround(%{{.*}}) : (f32) -> i32
- %3 = call i32 @llvm.lround.i32.f32(float %0)
- ret void
-}
|
|
|
|
Uff |
bcardosolopes
added a commit
to bcardosolopes/llvm-project
that referenced
this pull request
Feb 28, 2025
…sic_call Original introduced in llvm#128626, reverted in llvm#128973 Reproduced the issue on a shared lib build locally on Linux, moved content arround to statisfy both static and shared lib builds. Currently, the llvm importer can only cover intrinsics that have a first class representation in an MLIR dialect (arm-neon, etc). This PR introduces a fallback mechanism that allow "unregistered" intrinsics to be imported by using the generic `llvm.intrinsic_call` operation. This is useful in several ways: 1. Allows round-trip the LLVM dialect output lowered from other dialects (example: ClangIR) 2. Enables MLIR-linking tools to operate on imported LLVM IR without requiring to add new operations to dozen of different targets (cc @xlauko @smeenai). If multiple dialects implement this interface hook, the last one to register is the one converting all unregistered intrinsics.
bcardosolopes
added a commit
that referenced
this pull request
Mar 3, 2025
#129174) …sic_call Original introduced in #128626, reverted in #128973 Reproduced the issue on a shared lib build locally on Linux, moved content around to satisfy both static and shared lib builds. ### Original commit message Currently, the llvm importer can only cover intrinsics that have a first class representation in an MLIR dialect (arm-neon, etc). This PR introduces a fallback mechanism that allow "unregistered" intrinsics to be imported by using the generic `llvm.intrinsic_call` operation. This is useful in several ways: 1. Allows round-trip the LLVM dialect output lowered from other dialects (example: ClangIR) 2. Enables MLIR-linking tools to operate on imported LLVM IR without requiring to add new operations to dozen of different targets. If multiple dialects implement this interface hook, the last one to register is the one converting all unregistered intrinsics. --------- Co-authored-by: Bruno Cardoso Lopes <[email protected]>
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Mar 3, 2025
…llvm.intrin… (#129174) …sic_call Original introduced in llvm/llvm-project#128626, reverted in llvm/llvm-project#128973 Reproduced the issue on a shared lib build locally on Linux, moved content around to satisfy both static and shared lib builds. ### Original commit message Currently, the llvm importer can only cover intrinsics that have a first class representation in an MLIR dialect (arm-neon, etc). This PR introduces a fallback mechanism that allow "unregistered" intrinsics to be imported by using the generic `llvm.intrinsic_call` operation. This is useful in several ways: 1. Allows round-trip the LLVM dialect output lowered from other dialects (example: ClangIR) 2. Enables MLIR-linking tools to operate on imported LLVM IR without requiring to add new operations to dozen of different targets. If multiple dialects implement this interface hook, the last one to register is the one converting all unregistered intrinsics. --------- Co-authored-by: Bruno Cardoso Lopes <[email protected]>
joaosaffran
pushed a commit
to joaosaffran/llvm-project
that referenced
this pull request
Mar 3, 2025
…sic_call" (llvm#128973) Reverts llvm#128626 Looks like the static definition broke some bots! Co-authored-by: Bruno Cardoso Lopes <[email protected]>
jph-13
pushed a commit
to jph-13/llvm-project
that referenced
this pull request
Mar 21, 2025
llvm#129174) …sic_call Original introduced in llvm#128626, reverted in llvm#128973 Reproduced the issue on a shared lib build locally on Linux, moved content around to satisfy both static and shared lib builds. ### Original commit message Currently, the llvm importer can only cover intrinsics that have a first class representation in an MLIR dialect (arm-neon, etc). This PR introduces a fallback mechanism that allow "unregistered" intrinsics to be imported by using the generic `llvm.intrinsic_call` operation. This is useful in several ways: 1. Allows round-trip the LLVM dialect output lowered from other dialects (example: ClangIR) 2. Enables MLIR-linking tools to operate on imported LLVM IR without requiring to add new operations to dozen of different targets. If multiple dialects implement this interface hook, the last one to register is the one converting all unregistered intrinsics. --------- Co-authored-by: Bruno Cardoso Lopes <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reverts #128626
Looks like the static definition broke some bots!