[Mlir-commits] [mlir] [MLIR][LLVM] Add import flag to skip traversal of DICompositeType's elems (PR #89355)
Christian Ulmann
llvmlistbot at llvm.org
Fri Apr 19 05:02:06 PDT 2024
https://github.com/Dinistro updated https://github.com/llvm/llvm-project/pull/89355
>From 5ce54e478b6a03358c230f045b737851dbc600ff Mon Sep 17 00:00:00 2001
From: Christian Ulmann <christian.ulmann at nextsilicon.com>
Date: Fri, 19 Apr 2024 09:01:49 +0000
Subject: [PATCH 1/3] [MLIR][LLVM] Add import flag to skip traversal of
DICompositType's elems
This commit introduces a flag to allow skipping the potentially
recursive import of DICompositType elements. This patch is essentially a
bandaid for the still broken recursive debug type import.
Some of our downstream inputs are produced by excessive usage of
template meta programming, and thus contain tens of thousands of types
that all participate in such recursions. Unfortunately, the series of
patches that introduces type support is not easily revertable due to
being around for a while now and Modular depending on it.
---
mlir/include/mlir/Target/LLVMIR/Import.h | 13 ++++++----
.../include/mlir/Target/LLVMIR/ModuleImport.h | 2 +-
mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp | 8 +++++-
mlir/lib/Target/LLVMIR/DebugImporter.cpp | 14 +++++++----
mlir/lib/Target/LLVMIR/DebugImporter.h | 8 +++++-
mlir/lib/Target/LLVMIR/ModuleImport.cpp | 12 +++++----
.../Import/import-empty-di-composite-types.ll | 25 +++++++++++++++++++
7 files changed, 64 insertions(+), 18 deletions(-)
create mode 100644 mlir/test/Target/LLVMIR/Import/import-empty-di-composite-types.ll
diff --git a/mlir/include/mlir/Target/LLVMIR/Import.h b/mlir/include/mlir/Target/LLVMIR/Import.h
index 18c5c48bd9504b..2a4971012f9a56 100644
--- a/mlir/include/mlir/Target/LLVMIR/Import.h
+++ b/mlir/include/mlir/Target/LLVMIR/Import.h
@@ -14,8 +14,6 @@
#define MLIR_TARGET_LLVMIR_IMPORT_H
#include "mlir/IR/OwningOpRef.h"
-#include "mlir/Support/LLVM.h"
-#include "llvm/ADT/StringRef.h"
#include <memory>
// Forward-declare LLVM classes.
@@ -34,12 +32,17 @@ class ModuleOp;
/// The translation supports operations from any dialect that has a registered
/// implementation of the LLVMImportDialectInterface. It returns nullptr if the
/// translation fails and reports errors using the error handler registered with
-/// the MLIR context. The `emitExpensiveWarnings` option controls if expensive
+/// the MLIR context.
+/// The `emitExpensiveWarnings` option controls if expensive
/// but uncritical diagnostics should be emitted.
+/// The `importEmptyDICompositeTypes` option controls if DICompositeTypes should
+/// be imported without elements. This is a way to avoid triggering any parts of
+/// the recursive DI metadata import, which can consume substantial amounts of
+/// time.
OwningOpRef<ModuleOp>
translateLLVMIRToModule(std::unique_ptr<llvm::Module> llvmModule,
- MLIRContext *context,
- bool emitExpensiveWarnings = true);
+ MLIRContext *context, bool emitExpensiveWarnings = true,
+ bool importEmptyDICompositeTypes = false);
/// Translate the given LLVM data layout into an MLIR equivalent using the DLTI
/// dialect.
diff --git a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
index b551eb937cfe8d..1a188b1d042854 100644
--- a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
+++ b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
@@ -47,7 +47,7 @@ class LoopAnnotationImporter;
class ModuleImport {
public:
ModuleImport(ModuleOp mlirModule, std::unique_ptr<llvm::Module> llvmModule,
- bool emitExpensiveWarnings);
+ bool emitExpensiveWarnings, bool importEmptyDICompositeTypes);
/// Calls the LLVMImportInterface initialization that queries the registered
/// dialect interfaces for the supported LLVM IR intrinsics and metadata kinds
diff --git a/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp b/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
index c521d76a429958..8234fa4f32d8af 100644
--- a/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
+++ b/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
@@ -30,6 +30,11 @@ void registerFromLLVMIRTranslation() {
llvm::cl::desc("Emit expensive warnings during LLVM IR import "
"(discouraged: testing only!)"),
llvm::cl::init(false));
+ static llvm::cl::opt<bool> importEmptyDICompositeTypes(
+ "import-empty-di-composite-types",
+ llvm::cl::desc("Avoid translating the members of DICompositeTypes during "
+ "the LLVM IR import (discouraged: testing only!)"),
+ llvm::cl::init(false));
TranslateToMLIRRegistration registration(
"import-llvm", "Translate LLVMIR to MLIR",
@@ -51,7 +56,8 @@ void registerFromLLVMIRTranslation() {
return nullptr;
return translateLLVMIRToModule(std::move(llvmModule), context,
- emitExpensiveWarnings);
+ emitExpensiveWarnings,
+ importEmptyDICompositeTypes);
},
[](DialectRegistry ®istry) {
// Register the DLTI dialect used to express the data layout
diff --git a/mlir/lib/Target/LLVMIR/DebugImporter.cpp b/mlir/lib/Target/LLVMIR/DebugImporter.cpp
index 4a4e1d1ecdd868..297c80f7f842c5 100644
--- a/mlir/lib/Target/LLVMIR/DebugImporter.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugImporter.cpp
@@ -26,9 +26,11 @@ using namespace mlir;
using namespace mlir::LLVM;
using namespace mlir::LLVM::detail;
-DebugImporter::DebugImporter(ModuleOp mlirModule)
+DebugImporter::DebugImporter(ModuleOp mlirModule,
+ bool importEmptyDICompositeTypes)
: recursionPruner(mlirModule.getContext()),
- context(mlirModule.getContext()), mlirModule(mlirModule) {}
+ context(mlirModule.getContext()), mlirModule(mlirModule),
+ importEmptyDICompositeTypes(importEmptyDICompositeTypes) {}
Location DebugImporter::translateFuncLocation(llvm::Function *func) {
llvm::DISubprogram *subprogram = func->getSubprogram();
@@ -69,9 +71,11 @@ DICompileUnitAttr DebugImporter::translateImpl(llvm::DICompileUnit *node) {
DICompositeTypeAttr DebugImporter::translateImpl(llvm::DICompositeType *node) {
std::optional<DIFlags> flags = symbolizeDIFlags(node->getFlags());
SmallVector<DINodeAttr> elements;
- for (llvm::DINode *element : node->getElements()) {
- assert(element && "expected a non-null element type");
- elements.push_back(translate(element));
+ if (!importEmptyDICompositeTypes) {
+ for (llvm::DINode *element : node->getElements()) {
+ assert(element && "expected a non-null element type");
+ elements.push_back(translate(element));
+ }
}
// Drop the elements parameter if any of the elements are invalid.
if (llvm::is_contained(elements, nullptr))
diff --git a/mlir/lib/Target/LLVMIR/DebugImporter.h b/mlir/lib/Target/LLVMIR/DebugImporter.h
index 8b22dc63456775..8c34b88e71be88 100644
--- a/mlir/lib/Target/LLVMIR/DebugImporter.h
+++ b/mlir/lib/Target/LLVMIR/DebugImporter.h
@@ -29,7 +29,7 @@ namespace detail {
class DebugImporter {
public:
- DebugImporter(ModuleOp mlirModule);
+ DebugImporter(ModuleOp mlirModule, bool importEmptyDICompositeTypes);
/// Translates the given LLVM debug location to an MLIR location.
Location translateLoc(llvm::DILocation *loc);
@@ -184,6 +184,12 @@ class DebugImporter {
MLIRContext *context;
ModuleOp mlirModule;
+
+ /// An option to control if DICompositeTypes should always be imported without
+ /// converting their elements. This is a way to avoid recursive traversals of
+ /// types, which is currently still flawed for inputs produced by extensive
+ /// usage of template meta programming.
+ bool importEmptyDICompositeTypes;
};
} // namespace detail
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index af998b99d511f0..6255c7520789ba 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -155,12 +155,14 @@ getTopologicallySortedBlocks(ArrayRef<llvm::BasicBlock *> basicBlocks) {
ModuleImport::ModuleImport(ModuleOp mlirModule,
std::unique_ptr<llvm::Module> llvmModule,
- bool emitExpensiveWarnings)
+ bool emitExpensiveWarnings,
+ bool importEmptyDICompositeTypes)
: builder(mlirModule->getContext()), context(mlirModule->getContext()),
mlirModule(mlirModule), llvmModule(std::move(llvmModule)),
iface(mlirModule->getContext()),
typeTranslator(*mlirModule->getContext()),
- debugImporter(std::make_unique<DebugImporter>(mlirModule)),
+ debugImporter(std::make_unique<DebugImporter>(
+ mlirModule, importEmptyDICompositeTypes)),
loopAnnotationImporter(
std::make_unique<LoopAnnotationImporter>(*this, builder)),
emitExpensiveWarnings(emitExpensiveWarnings) {
@@ -2092,8 +2094,8 @@ ModuleImport::translateLoopAnnotationAttr(const llvm::MDNode *node,
OwningOpRef<ModuleOp>
mlir::translateLLVMIRToModule(std::unique_ptr<llvm::Module> llvmModule,
- MLIRContext *context,
- bool emitExpensiveWarnings) {
+ MLIRContext *context, bool emitExpensiveWarnings,
+ bool importEmptyDICompositeTypes) {
// 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
@@ -2109,7 +2111,7 @@ mlir::translateLLVMIRToModule(std::unique_ptr<llvm::Module> llvmModule,
/*column=*/0)));
ModuleImport moduleImport(module.get(), std::move(llvmModule),
- emitExpensiveWarnings);
+ emitExpensiveWarnings, importEmptyDICompositeTypes);
if (failed(moduleImport.initializeImportInterface()))
return {};
if (failed(moduleImport.convertDataLayout()))
diff --git a/mlir/test/Target/LLVMIR/Import/import-empty-di-composite-types.ll b/mlir/test/Target/LLVMIR/Import/import-empty-di-composite-types.ll
new file mode 100644
index 00000000000000..361a4779654f28
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/Import/import-empty-di-composite-types.ll
@@ -0,0 +1,25 @@
+; RUN: mlir-translate -import-llvm -mlir-print-debuginfo -split-input-file -import-empty-di-composite-types %s | FileCheck %s
+
+; Verifies that the "-import-empty-di-composite-types" flag avoids the
+; conversion of the elements of the DICompositeType.
+
+; CHECK-NOT: di_derive_type
+; CHECK: #{{.+}} = #llvm.di_composite_type<tag = DW_TAG_class_type, name = "class">
+; CHECK-NOT: di_derive_type
+
+define void @composite_type() !dbg !3 {
+ ret void
+}
+
+!llvm.dbg.cu = !{!1}
+!llvm.module.flags = !{!0}
+!0 = !{i32 2, !"Debug Info Version", i32 3}
+!1 = distinct !DICompileUnit(language: DW_LANG_C, file: !2)
+!2 = !DIFile(filename: "debug-info.ll", directory: "/")
+!3 = distinct !DISubprogram(name: "composite_type", scope: !2, file: !2, spFlags: DISPFlagDefinition, unit: !1, type: !4)
+!4 = !DISubroutineType(types: !5)
+!5 = !{!6}
+!6 = !DICompositeType(tag: DW_TAG_class_type, name: "class", elements: !7)
+!7 = !{!9}
+!8 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !6, flags: DIFlagArtificial | DIFlagObjectPointer)
+!9 = !DIDerivedType(tag: DW_TAG_member, name: "call_field", file: !2, baseType: !8)
>From a3eb7aeda14e71c23a0e365c5c8d4c520bb68a9d Mon Sep 17 00:00:00 2001
From: Christian Ulmann <christian.ulmann at nextsilicon.com>
Date: Fri, 19 Apr 2024 11:57:13 +0000
Subject: [PATCH 2/3] address review comments
---
mlir/include/mlir/Target/LLVMIR/Import.h | 10 +++++-----
mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp | 11 ++++++-----
mlir/lib/Target/LLVMIR/DebugImporter.cpp | 6 +++---
mlir/lib/Target/LLVMIR/DebugImporter.h | 10 +++++-----
mlir/lib/Target/LLVMIR/ModuleImport.cpp | 4 ++--
.../LLVMIR/Import/import-empty-di-composite-types.ll | 2 +-
6 files changed, 22 insertions(+), 21 deletions(-)
diff --git a/mlir/include/mlir/Target/LLVMIR/Import.h b/mlir/include/mlir/Target/LLVMIR/Import.h
index 2a4971012f9a56..4a1242a8eb0590 100644
--- a/mlir/include/mlir/Target/LLVMIR/Import.h
+++ b/mlir/include/mlir/Target/LLVMIR/Import.h
@@ -35,14 +35,14 @@ class ModuleOp;
/// the MLIR context.
/// The `emitExpensiveWarnings` option controls if expensive
/// but uncritical diagnostics should be emitted.
-/// The `importEmptyDICompositeTypes` option controls if DICompositeTypes should
-/// be imported without elements. This is a way to avoid triggering any parts of
-/// the recursive DI metadata import, which can consume substantial amounts of
-/// time.
+/// The `dropDICompositeTypeElements` option controls if DICompositeTypes should
+/// be imported without elements. If set, the option avoids the recursive
+/// traversal of composite type debug information, which can be expensive for
+/// adversarial inputs.
OwningOpRef<ModuleOp>
translateLLVMIRToModule(std::unique_ptr<llvm::Module> llvmModule,
MLIRContext *context, bool emitExpensiveWarnings = true,
- bool importEmptyDICompositeTypes = false);
+ bool dropDICompositeTypeElements = false);
/// Translate the given LLVM data layout into an MLIR equivalent using the DLTI
/// dialect.
diff --git a/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp b/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
index 8234fa4f32d8af..101add70c51e29 100644
--- a/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
+++ b/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
@@ -30,10 +30,11 @@ void registerFromLLVMIRTranslation() {
llvm::cl::desc("Emit expensive warnings during LLVM IR import "
"(discouraged: testing only!)"),
llvm::cl::init(false));
- static llvm::cl::opt<bool> importEmptyDICompositeTypes(
- "import-empty-di-composite-types",
- llvm::cl::desc("Avoid translating the members of DICompositeTypes during "
- "the LLVM IR import (discouraged: testing only!)"),
+ static llvm::cl::opt<bool> dropDICompositeTypeElements(
+ "drop-di-composite-type-elements",
+ llvm::cl::desc(
+ "Avoid translating the elements of DICompositeTypes during "
+ "the LLVM IR import (discouraged: testing only!)"),
llvm::cl::init(false));
TranslateToMLIRRegistration registration(
@@ -57,7 +58,7 @@ void registerFromLLVMIRTranslation() {
return translateLLVMIRToModule(std::move(llvmModule), context,
emitExpensiveWarnings,
- importEmptyDICompositeTypes);
+ dropDICompositeTypeElements);
},
[](DialectRegistry ®istry) {
// Register the DLTI dialect used to express the data layout
diff --git a/mlir/lib/Target/LLVMIR/DebugImporter.cpp b/mlir/lib/Target/LLVMIR/DebugImporter.cpp
index 297c80f7f842c5..b4c0115b0bca15 100644
--- a/mlir/lib/Target/LLVMIR/DebugImporter.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugImporter.cpp
@@ -27,10 +27,10 @@ using namespace mlir::LLVM;
using namespace mlir::LLVM::detail;
DebugImporter::DebugImporter(ModuleOp mlirModule,
- bool importEmptyDICompositeTypes)
+ bool dropDICompositeTypeElements)
: recursionPruner(mlirModule.getContext()),
context(mlirModule.getContext()), mlirModule(mlirModule),
- importEmptyDICompositeTypes(importEmptyDICompositeTypes) {}
+ dropDICompositeTypeElements(dropDICompositeTypeElements) {}
Location DebugImporter::translateFuncLocation(llvm::Function *func) {
llvm::DISubprogram *subprogram = func->getSubprogram();
@@ -71,7 +71,7 @@ DICompileUnitAttr DebugImporter::translateImpl(llvm::DICompileUnit *node) {
DICompositeTypeAttr DebugImporter::translateImpl(llvm::DICompositeType *node) {
std::optional<DIFlags> flags = symbolizeDIFlags(node->getFlags());
SmallVector<DINodeAttr> elements;
- if (!importEmptyDICompositeTypes) {
+ if (!dropDICompositeTypeElements) {
for (llvm::DINode *element : node->getElements()) {
assert(element && "expected a non-null element type");
elements.push_back(translate(element));
diff --git a/mlir/lib/Target/LLVMIR/DebugImporter.h b/mlir/lib/Target/LLVMIR/DebugImporter.h
index 8c34b88e71be88..5f402fb0b657cc 100644
--- a/mlir/lib/Target/LLVMIR/DebugImporter.h
+++ b/mlir/lib/Target/LLVMIR/DebugImporter.h
@@ -29,7 +29,7 @@ namespace detail {
class DebugImporter {
public:
- DebugImporter(ModuleOp mlirModule, bool importEmptyDICompositeTypes);
+ DebugImporter(ModuleOp mlirModule, bool dropDICompositeTypeElements);
/// Translates the given LLVM debug location to an MLIR location.
Location translateLoc(llvm::DILocation *loc);
@@ -186,10 +186,10 @@ class DebugImporter {
ModuleOp mlirModule;
/// An option to control if DICompositeTypes should always be imported without
- /// converting their elements. This is a way to avoid recursive traversals of
- /// types, which is currently still flawed for inputs produced by extensive
- /// usage of template meta programming.
- bool importEmptyDICompositeTypes;
+ /// converting their elements. If set, the option avoids the recursive
+ /// traversal of composite type debug information, which can be expensive for
+ /// adversarial inputs.
+ bool dropDICompositeTypeElements;
};
} // namespace detail
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index 6255c7520789ba..9d41fa0da37590 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -2095,7 +2095,7 @@ ModuleImport::translateLoopAnnotationAttr(const llvm::MDNode *node,
OwningOpRef<ModuleOp>
mlir::translateLLVMIRToModule(std::unique_ptr<llvm::Module> llvmModule,
MLIRContext *context, bool emitExpensiveWarnings,
- bool importEmptyDICompositeTypes) {
+ bool dropDICompositeTypeElements) {
// 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
@@ -2111,7 +2111,7 @@ mlir::translateLLVMIRToModule(std::unique_ptr<llvm::Module> llvmModule,
/*column=*/0)));
ModuleImport moduleImport(module.get(), std::move(llvmModule),
- emitExpensiveWarnings, importEmptyDICompositeTypes);
+ emitExpensiveWarnings, dropDICompositeTypeElements);
if (failed(moduleImport.initializeImportInterface()))
return {};
if (failed(moduleImport.convertDataLayout()))
diff --git a/mlir/test/Target/LLVMIR/Import/import-empty-di-composite-types.ll b/mlir/test/Target/LLVMIR/Import/import-empty-di-composite-types.ll
index 361a4779654f28..4caa6219c1feff 100644
--- a/mlir/test/Target/LLVMIR/Import/import-empty-di-composite-types.ll
+++ b/mlir/test/Target/LLVMIR/Import/import-empty-di-composite-types.ll
@@ -1,4 +1,4 @@
-; RUN: mlir-translate -import-llvm -mlir-print-debuginfo -split-input-file -import-empty-di-composite-types %s | FileCheck %s
+; RUN: mlir-translate -import-llvm -mlir-print-debuginfo -split-input-file -drop-di-composite-type-elements %s | FileCheck %s
; Verifies that the "-import-empty-di-composite-types" flag avoids the
; conversion of the elements of the DICompositeType.
>From 26f206adb60c6f6b48dae9b19c3b7251e2177533 Mon Sep 17 00:00:00 2001
From: Christian Ulmann <christian.ulmann at nextsilicon.com>
Date: Fri, 19 Apr 2024 12:01:51 +0000
Subject: [PATCH 3/3] fix flag name remnants
---
...i-composite-types.ll => ignore-composite-type-elements.ll} | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
rename mlir/test/Target/LLVMIR/Import/{import-empty-di-composite-types.ll => ignore-composite-type-elements.ll} (88%)
diff --git a/mlir/test/Target/LLVMIR/Import/import-empty-di-composite-types.ll b/mlir/test/Target/LLVMIR/Import/ignore-composite-type-elements.ll
similarity index 88%
rename from mlir/test/Target/LLVMIR/Import/import-empty-di-composite-types.ll
rename to mlir/test/Target/LLVMIR/Import/ignore-composite-type-elements.ll
index 4caa6219c1feff..a249f2290d7ae1 100644
--- a/mlir/test/Target/LLVMIR/Import/import-empty-di-composite-types.ll
+++ b/mlir/test/Target/LLVMIR/Import/ignore-composite-type-elements.ll
@@ -1,7 +1,7 @@
; RUN: mlir-translate -import-llvm -mlir-print-debuginfo -split-input-file -drop-di-composite-type-elements %s | FileCheck %s
-; Verifies that the "-import-empty-di-composite-types" flag avoids the
-; conversion of the elements of the DICompositeType.
+; Verifies that the according flag avoids the conversion of the elements of the
+; DICompositeType.
; CHECK-NOT: di_derive_type
; CHECK: #{{.+}} = #llvm.di_composite_type<tag = DW_TAG_class_type, name = "class">
More information about the Mlir-commits
mailing list