[MLIR][LLVMIR] Import: add flag to prefer using unregistered intrinsics #130685
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
|
@llvm/pr-subscribers-mlir-llvm Author: Bruno Cardoso Lopes (bcardosolopes) ChangesCurrently, there is no common mechanism for supported intrinsics to be generically annotated with arg and ret attributes. Since there are many supported intrinsics around different dialects, the amount of work to teach all them about these attributes is not trivial (though it would be nice in the long term). This PR adds a new flag Using this flag allow us to roundtrip the LLVM IR while eliminating a whole set of differences coming from lack of arg/ret attributes on supported intrinsics. Note Full diff: https://github.com/llvm/llvm-project/pull/130685.diff 7 Files Affected:
diff --git a/mlir/include/mlir/Target/LLVMIR/Import.h b/mlir/include/mlir/Target/LLVMIR/Import.h
index 4aa8f2ab7d8ce..2fef44947b6c2 100644
--- a/mlir/include/mlir/Target/LLVMIR/Import.h
+++ b/mlir/include/mlir/Target/LLVMIR/Import.h
@@ -41,11 +41,13 @@ class ModuleOp;
/// adversarial inputs.
/// The `loadAllDialects` flag (default on) will load all dialects in the
/// context.
-OwningOpRef<ModuleOp>
-translateLLVMIRToModule(std::unique_ptr<llvm::Module> llvmModule,
- MLIRContext *context, bool emitExpensiveWarnings = true,
- bool dropDICompositeTypeElements = false,
- bool loadAllDialects = true);
+/// The `preferUnregisteredIntrinsics` flag (default off) controls whether to
+/// prefer generic version of imported intrinsics with `llvm.intrinsic_call`
+/// than using versions supported by a dialects.
+OwningOpRef<ModuleOp> translateLLVMIRToModule(
+ std::unique_ptr<llvm::Module> llvmModule, MLIRContext *context,
+ bool emitExpensiveWarnings = true, bool dropDICompositeTypeElements = false,
+ bool loadAllDialects = true, bool preferUnregisteredIntrinsics = false);
/// Translate the given LLVM data layout into an MLIR equivalent using the DLTI
/// dialect.
diff --git a/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h b/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h
index 686969f891f20..6a42627e17e60 100644
--- a/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h
+++ b/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h
@@ -153,26 +153,7 @@ class LLVMImportInterface
/// Converts the LLVM intrinsic to an MLIR operation if a conversion exists.
/// Returns failure otherwise.
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).
- if (!dialect)
- return convertUnregisteredIntrinsic(builder, inst, moduleImport);
-
- // Dispatch the conversion to the dialect interface.
- const LLVMImportDialectInterface *iface = getInterfaceFor(dialect);
- assert(iface && "expected to find a dialect interface");
- return iface->convertIntrinsic(builder, inst, moduleImport);
- }
+ LLVM::ModuleImport &moduleImport) const;
/// Returns true if the given LLVM IR intrinsic is convertible to an MLIR
/// operation.
diff --git a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
index c0af924e0aecd..3e239e0112afc 100644
--- a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
+++ b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
@@ -47,7 +47,8 @@ class LoopAnnotationImporter;
class ModuleImport {
public:
ModuleImport(ModuleOp mlirModule, std::unique_ptr<llvm::Module> llvmModule,
- bool emitExpensiveWarnings, bool importEmptyDICompositeTypes);
+ bool emitExpensiveWarnings, bool importEmptyDICompositeTypes,
+ bool preferUnregisteredIntrinsics);
/// Calls the LLVMImportInterface initialization that queries the registered
/// dialect interfaces for the supported LLVM IR intrinsics and metadata kinds
@@ -277,6 +278,12 @@ class ModuleImport {
void convertParameterAttributes(llvm::CallBase *call, ArrayAttr &argsAttr,
ArrayAttr &resAttr, OpBuilder &builder);
+ /// Whether the importer should try to convert all intrinsics to
+ /// llvm.call_intrinsic instead of dialect supported operations.
+ bool useUnregisteredIntrinsicsOnly() const {
+ return preferUnregisteredIntrinsics;
+ }
+
private:
/// Clears the accumulated state before processing a new region.
void clearRegionState() {
@@ -474,6 +481,10 @@ class ModuleImport {
/// emitted. Avoids generating warnings for unhandled debug intrinsics and
/// metadata that otherwise dominate the translation time for large inputs.
bool emitExpensiveWarnings;
+
+ /// An option to control whether to disable supported intrinsic support in
+ /// favor a more generic version via `llvm.intrinsic_call`.
+ bool preferUnregisteredIntrinsics;
};
} // namespace LLVM
diff --git a/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp b/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
index 0784c3c95e47e..b10fdd6f37f79 100644
--- a/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
+++ b/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
@@ -37,6 +37,13 @@ void registerFromLLVMIRTranslation() {
"the LLVM IR import (discouraged: testing only!)"),
llvm::cl::init(false));
+ static llvm::cl::opt<bool> preferUnregisteredIntrinsics(
+ "prefer-unregistered-intrinsics",
+ llvm::cl::desc(
+ "Prefer translation all intrinsics into llvm.call_intrinsic instead "
+ "of using dialect supported intrinsics"),
+ llvm::cl::init(false));
+
TranslateToMLIRRegistration registration(
"import-llvm", "Translate LLVMIR to MLIR",
[](llvm::SourceMgr &sourceMgr,
@@ -60,9 +67,10 @@ void registerFromLLVMIRTranslation() {
if (llvmModule->IsNewDbgInfoFormat)
llvmModule->convertFromNewDbgValues();
- return translateLLVMIRToModule(std::move(llvmModule), context,
- emitExpensiveWarnings,
- dropDICompositeTypeElements);
+ return translateLLVMIRToModule(
+ std::move(llvmModule), context, emitExpensiveWarnings,
+ dropDICompositeTypeElements, /*loadAllDialects=*/true,
+ preferUnregisteredIntrinsics);
},
[](DialectRegistry ®istry) {
// Register the DLTI dialect used to express the data layout
diff --git a/mlir/lib/Target/LLVMIR/LLVMImportInterface.cpp b/mlir/lib/Target/LLVMIR/LLVMImportInterface.cpp
index fbf2d709c240c..2d7257a2d9698 100644
--- a/mlir/lib/Target/LLVMIR/LLVMImportInterface.cpp
+++ b/mlir/lib/Target/LLVMIR/LLVMImportInterface.cpp
@@ -62,3 +62,31 @@ LogicalResult mlir::LLVMImportInterface::convertUnregisteredIntrinsic(
return success();
}
+
+/// Converts the LLVM intrinsic to an MLIR operation if a conversion exists.
+/// Returns failure otherwise.
+LogicalResult mlir::LLVMImportInterface::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 = nullptr;
+
+ if (!moduleImport.useUnregisteredIntrinsicsOnly())
+ dialect = intrinsicToDialect.lookup(intrinId);
+
+ // No specialized (supported) intrinsics, attempt to generate a generic
+ // version via llvm.call_intrinsic (if available).
+ if (!dialect)
+ return convertUnregisteredIntrinsic(builder, inst, moduleImport);
+
+ // Dispatch the conversion to the dialect interface.
+ const LLVMImportDialectInterface *iface = getInterfaceFor(dialect);
+ assert(iface && "expected to find a dialect interface");
+ return iface->convertIntrinsic(builder, inst, moduleImport);
+}
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index f24c2777cbbb8..0f156779b46b5 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -162,7 +162,8 @@ getTopologicallySortedBlocks(ArrayRef<llvm::BasicBlock *> basicBlocks) {
ModuleImport::ModuleImport(ModuleOp mlirModule,
std::unique_ptr<llvm::Module> llvmModule,
bool emitExpensiveWarnings,
- bool importEmptyDICompositeTypes)
+ bool importEmptyDICompositeTypes,
+ bool preferUnregisteredIntrinsics)
: builder(mlirModule->getContext()), context(mlirModule->getContext()),
mlirModule(mlirModule), llvmModule(std::move(llvmModule)),
iface(mlirModule->getContext()),
@@ -171,7 +172,8 @@ ModuleImport::ModuleImport(ModuleOp mlirModule,
mlirModule, importEmptyDICompositeTypes)),
loopAnnotationImporter(
std::make_unique<LoopAnnotationImporter>(*this, builder)),
- emitExpensiveWarnings(emitExpensiveWarnings) {
+ emitExpensiveWarnings(emitExpensiveWarnings),
+ preferUnregisteredIntrinsics(preferUnregisteredIntrinsics) {
builder.setInsertionPointToStart(mlirModule.getBody());
}
@@ -2527,11 +2529,10 @@ ModuleImport::translateLoopAnnotationAttr(const llvm::MDNode *node,
return loopAnnotationImporter->translateLoopAnnotation(node, loc);
}
-OwningOpRef<ModuleOp>
-mlir::translateLLVMIRToModule(std::unique_ptr<llvm::Module> llvmModule,
- MLIRContext *context, bool emitExpensiveWarnings,
- bool dropDICompositeTypeElements,
- bool loadAllDialects) {
+OwningOpRef<ModuleOp> mlir::translateLLVMIRToModule(
+ std::unique_ptr<llvm::Module> llvmModule, MLIRContext *context,
+ bool emitExpensiveWarnings, bool dropDICompositeTypeElements,
+ bool loadAllDialects, bool preferUnregisteredIntrinsics) {
// Preload all registered dialects to allow the import to iterate the
// registered LLVMImportDialectInterface implementations and query the
// supported LLVM IR constructs before starting the translation. Assumes the
@@ -2548,7 +2549,8 @@ mlir::translateLLVMIRToModule(std::unique_ptr<llvm::Module> llvmModule,
/*column=*/0)));
ModuleImport moduleImport(module.get(), std::move(llvmModule),
- emitExpensiveWarnings, dropDICompositeTypeElements);
+ emitExpensiveWarnings, dropDICompositeTypeElements,
+ preferUnregisteredIntrinsics);
if (failed(moduleImport.initializeImportInterface()))
return {};
if (failed(moduleImport.convertDataLayout()))
diff --git a/mlir/test/Target/LLVMIR/Import/intrinsic-prefer-unregistered.ll b/mlir/test/Target/LLVMIR/Import/intrinsic-prefer-unregistered.ll
new file mode 100644
index 0000000000000..778289469c4a5
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/Import/intrinsic-prefer-unregistered.ll
@@ -0,0 +1,10 @@
+; RUN: mlir-translate -import-llvm -prefer-unregistered-intrinsics %s | FileCheck %s
+
+; CHECK-LABEL: llvm.func @lifetime
+define void @lifetime(ptr %0) {
+ ; CHECK: llvm.call_intrinsic "llvm.lifetime.start.p0"({{.*}}, %arg0) : (i64, !llvm.ptr {llvm.nonnull}) -> !llvm.void
+ call void @llvm.lifetime.start.p0(i64 16, ptr nonnull %0)
+ ; CHECK: llvm.call_intrinsic "llvm.lifetime.end.p0"({{.*}}, %arg0) : (i64, !llvm.ptr {llvm.nonnull}) -> !llvm.void
+ call void @llvm.lifetime.end.p0(i64 32, ptr nonnull %0)
+ ret void
+}
|
|
@llvm/pr-subscribers-mlir Author: Bruno Cardoso Lopes (bcardosolopes) ChangesCurrently, there is no common mechanism for supported intrinsics to be generically annotated with arg and ret attributes. Since there are many supported intrinsics around different dialects, the amount of work to teach all them about these attributes is not trivial (though it would be nice in the long term). This PR adds a new flag Using this flag allow us to roundtrip the LLVM IR while eliminating a whole set of differences coming from lack of arg/ret attributes on supported intrinsics. Note Full diff: https://github.com/llvm/llvm-project/pull/130685.diff 7 Files Affected:
diff --git a/mlir/include/mlir/Target/LLVMIR/Import.h b/mlir/include/mlir/Target/LLVMIR/Import.h
index 4aa8f2ab7d8ce..2fef44947b6c2 100644
--- a/mlir/include/mlir/Target/LLVMIR/Import.h
+++ b/mlir/include/mlir/Target/LLVMIR/Import.h
@@ -41,11 +41,13 @@ class ModuleOp;
/// adversarial inputs.
/// The `loadAllDialects` flag (default on) will load all dialects in the
/// context.
-OwningOpRef<ModuleOp>
-translateLLVMIRToModule(std::unique_ptr<llvm::Module> llvmModule,
- MLIRContext *context, bool emitExpensiveWarnings = true,
- bool dropDICompositeTypeElements = false,
- bool loadAllDialects = true);
+/// The `preferUnregisteredIntrinsics` flag (default off) controls whether to
+/// prefer generic version of imported intrinsics with `llvm.intrinsic_call`
+/// than using versions supported by a dialects.
+OwningOpRef<ModuleOp> translateLLVMIRToModule(
+ std::unique_ptr<llvm::Module> llvmModule, MLIRContext *context,
+ bool emitExpensiveWarnings = true, bool dropDICompositeTypeElements = false,
+ bool loadAllDialects = true, bool preferUnregisteredIntrinsics = false);
/// Translate the given LLVM data layout into an MLIR equivalent using the DLTI
/// dialect.
diff --git a/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h b/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h
index 686969f891f20..6a42627e17e60 100644
--- a/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h
+++ b/mlir/include/mlir/Target/LLVMIR/LLVMImportInterface.h
@@ -153,26 +153,7 @@ class LLVMImportInterface
/// Converts the LLVM intrinsic to an MLIR operation if a conversion exists.
/// Returns failure otherwise.
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).
- if (!dialect)
- return convertUnregisteredIntrinsic(builder, inst, moduleImport);
-
- // Dispatch the conversion to the dialect interface.
- const LLVMImportDialectInterface *iface = getInterfaceFor(dialect);
- assert(iface && "expected to find a dialect interface");
- return iface->convertIntrinsic(builder, inst, moduleImport);
- }
+ LLVM::ModuleImport &moduleImport) const;
/// Returns true if the given LLVM IR intrinsic is convertible to an MLIR
/// operation.
diff --git a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
index c0af924e0aecd..3e239e0112afc 100644
--- a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
+++ b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
@@ -47,7 +47,8 @@ class LoopAnnotationImporter;
class ModuleImport {
public:
ModuleImport(ModuleOp mlirModule, std::unique_ptr<llvm::Module> llvmModule,
- bool emitExpensiveWarnings, bool importEmptyDICompositeTypes);
+ bool emitExpensiveWarnings, bool importEmptyDICompositeTypes,
+ bool preferUnregisteredIntrinsics);
/// Calls the LLVMImportInterface initialization that queries the registered
/// dialect interfaces for the supported LLVM IR intrinsics and metadata kinds
@@ -277,6 +278,12 @@ class ModuleImport {
void convertParameterAttributes(llvm::CallBase *call, ArrayAttr &argsAttr,
ArrayAttr &resAttr, OpBuilder &builder);
+ /// Whether the importer should try to convert all intrinsics to
+ /// llvm.call_intrinsic instead of dialect supported operations.
+ bool useUnregisteredIntrinsicsOnly() const {
+ return preferUnregisteredIntrinsics;
+ }
+
private:
/// Clears the accumulated state before processing a new region.
void clearRegionState() {
@@ -474,6 +481,10 @@ class ModuleImport {
/// emitted. Avoids generating warnings for unhandled debug intrinsics and
/// metadata that otherwise dominate the translation time for large inputs.
bool emitExpensiveWarnings;
+
+ /// An option to control whether to disable supported intrinsic support in
+ /// favor a more generic version via `llvm.intrinsic_call`.
+ bool preferUnregisteredIntrinsics;
};
} // namespace LLVM
diff --git a/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp b/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
index 0784c3c95e47e..b10fdd6f37f79 100644
--- a/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
+++ b/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
@@ -37,6 +37,13 @@ void registerFromLLVMIRTranslation() {
"the LLVM IR import (discouraged: testing only!)"),
llvm::cl::init(false));
+ static llvm::cl::opt<bool> preferUnregisteredIntrinsics(
+ "prefer-unregistered-intrinsics",
+ llvm::cl::desc(
+ "Prefer translation all intrinsics into llvm.call_intrinsic instead "
+ "of using dialect supported intrinsics"),
+ llvm::cl::init(false));
+
TranslateToMLIRRegistration registration(
"import-llvm", "Translate LLVMIR to MLIR",
[](llvm::SourceMgr &sourceMgr,
@@ -60,9 +67,10 @@ void registerFromLLVMIRTranslation() {
if (llvmModule->IsNewDbgInfoFormat)
llvmModule->convertFromNewDbgValues();
- return translateLLVMIRToModule(std::move(llvmModule), context,
- emitExpensiveWarnings,
- dropDICompositeTypeElements);
+ return translateLLVMIRToModule(
+ std::move(llvmModule), context, emitExpensiveWarnings,
+ dropDICompositeTypeElements, /*loadAllDialects=*/true,
+ preferUnregisteredIntrinsics);
},
[](DialectRegistry ®istry) {
// Register the DLTI dialect used to express the data layout
diff --git a/mlir/lib/Target/LLVMIR/LLVMImportInterface.cpp b/mlir/lib/Target/LLVMIR/LLVMImportInterface.cpp
index fbf2d709c240c..2d7257a2d9698 100644
--- a/mlir/lib/Target/LLVMIR/LLVMImportInterface.cpp
+++ b/mlir/lib/Target/LLVMIR/LLVMImportInterface.cpp
@@ -62,3 +62,31 @@ LogicalResult mlir::LLVMImportInterface::convertUnregisteredIntrinsic(
return success();
}
+
+/// Converts the LLVM intrinsic to an MLIR operation if a conversion exists.
+/// Returns failure otherwise.
+LogicalResult mlir::LLVMImportInterface::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 = nullptr;
+
+ if (!moduleImport.useUnregisteredIntrinsicsOnly())
+ dialect = intrinsicToDialect.lookup(intrinId);
+
+ // No specialized (supported) intrinsics, attempt to generate a generic
+ // version via llvm.call_intrinsic (if available).
+ if (!dialect)
+ return convertUnregisteredIntrinsic(builder, inst, moduleImport);
+
+ // Dispatch the conversion to the dialect interface.
+ const LLVMImportDialectInterface *iface = getInterfaceFor(dialect);
+ assert(iface && "expected to find a dialect interface");
+ return iface->convertIntrinsic(builder, inst, moduleImport);
+}
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index f24c2777cbbb8..0f156779b46b5 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -162,7 +162,8 @@ getTopologicallySortedBlocks(ArrayRef<llvm::BasicBlock *> basicBlocks) {
ModuleImport::ModuleImport(ModuleOp mlirModule,
std::unique_ptr<llvm::Module> llvmModule,
bool emitExpensiveWarnings,
- bool importEmptyDICompositeTypes)
+ bool importEmptyDICompositeTypes,
+ bool preferUnregisteredIntrinsics)
: builder(mlirModule->getContext()), context(mlirModule->getContext()),
mlirModule(mlirModule), llvmModule(std::move(llvmModule)),
iface(mlirModule->getContext()),
@@ -171,7 +172,8 @@ ModuleImport::ModuleImport(ModuleOp mlirModule,
mlirModule, importEmptyDICompositeTypes)),
loopAnnotationImporter(
std::make_unique<LoopAnnotationImporter>(*this, builder)),
- emitExpensiveWarnings(emitExpensiveWarnings) {
+ emitExpensiveWarnings(emitExpensiveWarnings),
+ preferUnregisteredIntrinsics(preferUnregisteredIntrinsics) {
builder.setInsertionPointToStart(mlirModule.getBody());
}
@@ -2527,11 +2529,10 @@ ModuleImport::translateLoopAnnotationAttr(const llvm::MDNode *node,
return loopAnnotationImporter->translateLoopAnnotation(node, loc);
}
-OwningOpRef<ModuleOp>
-mlir::translateLLVMIRToModule(std::unique_ptr<llvm::Module> llvmModule,
- MLIRContext *context, bool emitExpensiveWarnings,
- bool dropDICompositeTypeElements,
- bool loadAllDialects) {
+OwningOpRef<ModuleOp> mlir::translateLLVMIRToModule(
+ std::unique_ptr<llvm::Module> llvmModule, MLIRContext *context,
+ bool emitExpensiveWarnings, bool dropDICompositeTypeElements,
+ bool loadAllDialects, bool preferUnregisteredIntrinsics) {
// Preload all registered dialects to allow the import to iterate the
// registered LLVMImportDialectInterface implementations and query the
// supported LLVM IR constructs before starting the translation. Assumes the
@@ -2548,7 +2549,8 @@ mlir::translateLLVMIRToModule(std::unique_ptr<llvm::Module> llvmModule,
/*column=*/0)));
ModuleImport moduleImport(module.get(), std::move(llvmModule),
- emitExpensiveWarnings, dropDICompositeTypeElements);
+ emitExpensiveWarnings, dropDICompositeTypeElements,
+ preferUnregisteredIntrinsics);
if (failed(moduleImport.initializeImportInterface()))
return {};
if (failed(moduleImport.convertDataLayout()))
diff --git a/mlir/test/Target/LLVMIR/Import/intrinsic-prefer-unregistered.ll b/mlir/test/Target/LLVMIR/Import/intrinsic-prefer-unregistered.ll
new file mode 100644
index 0000000000000..778289469c4a5
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/Import/intrinsic-prefer-unregistered.ll
@@ -0,0 +1,10 @@
+; RUN: mlir-translate -import-llvm -prefer-unregistered-intrinsics %s | FileCheck %s
+
+; CHECK-LABEL: llvm.func @lifetime
+define void @lifetime(ptr %0) {
+ ; CHECK: llvm.call_intrinsic "llvm.lifetime.start.p0"({{.*}}, %arg0) : (i64, !llvm.ptr {llvm.nonnull}) -> !llvm.void
+ call void @llvm.lifetime.start.p0(i64 16, ptr nonnull %0)
+ ; CHECK: llvm.call_intrinsic "llvm.lifetime.end.p0"({{.*}}, %arg0) : (i64, !llvm.ptr {llvm.nonnull}) -> !llvm.void
+ call void @llvm.lifetime.end.p0(i64 32, ptr nonnull %0)
+ ret void
+}
|
gysit
reviewed
Mar 11, 2025
There was a problem hiding this comment.
I think we need to put a fat warning on the flag since this may fail if passes rely on detecting the explicit intrinsic operation.
I also wonder if we should move the handling of the unregistered intrinsics in ModuleImport? It feels like the right place given we need to leak more an more information to the interface. Or is there a need to handle them in the interface?
Do you mean emitting an warning or just writing a warning in a comment? If the former: we might want to use LLVM IR without the supported intrinsics in production (i.e. not only for testing) - in which case the warning feels like a constant annoyance / misleading. Should I introduce an extra flag for controlling such warning?
Hmm, while working on the PR that added the unregistered intrinsics you asked similar and I couldn't easily make it happen - there were complications in the interaction between querying other dialects via interfaces along side the fallback mechanism which is LLVM dialect only. It's probably possible but might need moving enough pieces around that should be done in its own PR. |
I meant just writing a warning in the comment (basically something like the one I suggested above)! It is a potential foot gun if you do transformations on the IR that expect explicit intrinsics. |
Couldn't interface do the following:
The module import could then:
I would expect that this works quite smoothly except there is some issue with the error handling. I coming back to it bcs the interface should usually do the dialect specific stuff while the import itself contains most of the LLVM IR specific stuff. Anyway let's postpone to a later pr. |
|
Created an issue to track next steps. Fixed comment and added a warning. |
gysit
approved these changes
Mar 14, 2025
Currently, there is no common mechanism for supported intrinsics to be generically annotated with arg and ret attributes. Since there are many supported intrinsics around different dialects, the amount of work to teach all them about these attributes is not trivial. This PR adds a new flag `-prefer-unregistered-intrinsics` that can be used alongside `--import-llvm` to always use `llvm.intrinsic_call` during import time (ignoring dialect hooks for custom intrinsic support). Using this flag allow us to roundtrip the LLVM IR while eliminating a whole set of differences coming from lack of arg/ret attributes on supported intrinsics. Note `convertIntrinsic` has to be moved to an implementation file because it queries into `moduleImport` state, which is a fwd declaration in `LLVMImportInterface.h`
bcardosolopes
force-pushed
the
mlir-llvm-prefer-unreg-intrin
branch
from
0935d0e to
201ff57
Compare
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.
Currently, there is no common mechanism for supported intrinsics to be generically annotated with arg and ret attributes. Since there are many supported intrinsics around different dialects, the amount of work to teach all them about these attributes is not trivial (though it would be nice in the long term).
This PR adds a new flag
-prefer-unregistered-intrinsicsthat can be used alongside--import-llvmto always usellvm.intrinsic_callduring import time (ignoring dialect hooks for custom intrinsic support).Using this flag allow us to roundtrip the LLVM IR while eliminating a whole set of differences coming from lack of arg/ret attributes on supported intrinsics.
Note
convertIntrinsichas to be moved to an implementation file because it queries intomoduleImportstate, which is a fwd declaration inLLVMImportInterface.h