[Mlir-commits] [mlir] [MLIR][LLVM] Add import flag to skip traversal of DICompositType's elems (PR #89355)

llvmlistbot at llvm.org llvmlistbot at llvm.org
Fri Apr 19 02:12:09 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-llvm

Author: Christian Ulmann (Dinistro)

<details>
<summary>Changes</summary>

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.

We can consider to revert this change once the type importer has show to be very performant, but for now we are talking second vs hours to import specific files.

---
Full diff: https://github.com/llvm/llvm-project/pull/89355.diff


7 Files Affected:

- (modified) mlir/include/mlir/Target/LLVMIR/Import.h (+8-5) 
- (modified) mlir/include/mlir/Target/LLVMIR/ModuleImport.h (+1-1) 
- (modified) mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp (+7-1) 
- (modified) mlir/lib/Target/LLVMIR/DebugImporter.cpp (+9-5) 
- (modified) mlir/lib/Target/LLVMIR/DebugImporter.h (+7-1) 
- (modified) mlir/lib/Target/LLVMIR/ModuleImport.cpp (+7-5) 
- (added) mlir/test/Target/LLVMIR/Import/import-empty-di-composite-types.ll (+25) 


``````````diff
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 &registry) {
         // 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)

``````````

</details>


https://github.com/llvm/llvm-project/pull/89355


More information about the Mlir-commits mailing list