[MLIR][LLVM] Avoid duplicated module flags in the export (#131627)

This commit resolves an issue in the LLVMIR export that caused the
duplication of the "Debug Info Version" module flag, when it was already
in MLIR.

Author: Dinistro

mlir/lib/Target/LLVMIR/DebugTranslation.cpp

@@ -32,31 +32,29 @@ DebugTranslation::DebugTranslation(Operation *module, llvm::Module &llvmModule)
   if (!module->walk(interruptIfValidLocation).wasInterrupted())
     return;
   debugEmissionIsEnabled = true;
+}
+
+static constexpr StringRef kDebugVersionKey = "Debug Info Version";
+static constexpr StringRef kCodeViewKey = "CodeView";
 
+void DebugTranslation::addModuleFlagsIfNotPresent() {
   // TODO: The version information should be encoded on the LLVM module itself,
   // not implicitly set here.
 
   // Mark this module as having debug information.
-  StringRef debugVersionKey = "Debug Info Version";
-  if (!llvmModule.getModuleFlag(debugVersionKey))
-    llvmModule.addModuleFlag(llvm::Module::Warning, debugVersionKey,
+  if (!llvmModule.getModuleFlag(kDebugVersionKey))
+    llvmModule.addModuleFlag(llvm::Module::Warning, kDebugVersionKey,
                              llvm::DEBUG_METADATA_VERSION);
 
-  if (auto targetTripleAttr = module->getDiscardableAttr(
-          LLVM::LLVMDialect::getTargetTripleAttrName())) {
-    auto targetTriple =
-        llvm::Triple(cast<StringAttr>(targetTripleAttr).getValue());
-    if (targetTriple.isKnownWindowsMSVCEnvironment()) {
-      // Dwarf debugging files will be generated by default, unless "CodeView"
-      // is set explicitly. Windows/MSVC should use CodeView instead.
-      llvmModule.addModuleFlag(llvm::Module::Warning, "CodeView", 1);
-    }
+  const llvm::Triple &targetTriple = llvmModule.getTargetTriple();
+  if (targetTriple.isKnownWindowsMSVCEnvironment()) {
+    // Dwarf debugging files will be generated by default, unless "CodeView"
+    // is set explicitly. Windows/MSVC should use CodeView instead.
+    if (!llvmModule.getModuleFlag(kCodeViewKey))
+      llvmModule.addModuleFlag(llvm::Module::Warning, kCodeViewKey, 1);
   }
 }
 
-/// Finalize the translation of debug information.
-void DebugTranslation::finalize() {}
-
 /// Translate the debug information for the given function.
 void DebugTranslation::translate(LLVMFuncOp func, llvm::Function &llvmFunc) {
   if (!debugEmissionIsEnabled)

mlir/lib/Target/LLVMIR/DebugTranslation.h

@@ -31,8 +31,8 @@ class DebugTranslation {
 public:
   DebugTranslation(Operation *module, llvm::Module &llvmModule);
 
-  /// Finalize the translation of debug information.
-  void finalize();
+  /// Adds the necessary module flags to the module, if not yet present.
+  void addModuleFlagsIfNotPresent();
 
   /// Translate the given location to an llvm debug location.
   llvm::DILocation *translateLoc(Location loc, llvm::DILocalScope *scope);

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

@@ -2223,6 +2223,10 @@ mlir::translateModuleToLLVMIR(Operation *module, llvm::LLVMContext &llvmContext,
   // See https://llvm.org/docs/RemoveDIsDebugInfo.html
   translator.llvmModule->setIsNewDbgInfoFormat(UseNewDbgInfoFormat);
 
+  // Add the necessary debug info module flags, if they were not encoded in MLIR
+  // beforehand.
+  translator.debugTranslation->addModuleFlagsIfNotPresent();
+
   if (!disableVerification &&
       llvm::verifyModule(*translator.llvmModule, &llvm::errs()))
     return nullptr;

mlir/test/Target/LLVMIR/llvmir-debug.mlir

@@ -316,8 +316,8 @@ llvm.func @dbg_intrinsics_with_no_location(%arg0: i32) -> (i32) {
 
 // CHECK: @global_with_expr_1 = external global i64, !dbg {{.*}}
 // CHECK: @global_with_expr_2 = external global i64, !dbg {{.*}}
-// CHECK: !llvm.module.flags = !{{{.*}}}
-// CHECK: !llvm.dbg.cu = !{{{.*}}}
+// CHECK-DAG: !llvm.module.flags = !{{{.*}}}
+// CHECK-DAG: !llvm.dbg.cu = !{{{.*}}}
 // CHECK-DAG: ![[FILE:.*]] = !DIFile(filename: "not", directory: "existence")
 // CHECK-DAG: ![[TYPE:.*]] = !DIBasicType(name: "uint64_t", size: 64, encoding: DW_ATE_unsigned)
 // CHECK-DAG: ![[SCOPE:.*]] = distinct !DICompileUnit(language: DW_LANG_C, file: ![[FILE]], producer: "MLIR", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, globals: ![[GVALS:.*]])
@@ -336,8 +336,8 @@ llvm.mlir.global external @global_with_expr_2() {addr_space = 0 : i32, dbg_exprs
 // -----
 
 // CHECK: @module_global = external global i64, !dbg {{.*}}
-// CHECK: !llvm.module.flags = !{{{.*}}}
-// CHECK: !llvm.dbg.cu = !{{{.*}}}
+// CHECK-DAG: !llvm.module.flags = !{{{.*}}}
+// CHECK-DAG: !llvm.dbg.cu = !{{{.*}}}
 // CHECK-DAG: ![[FILE:.*]] = !DIFile(filename: "test.f90", directory: "existence")
 // CHECK-DAG: ![[TYPE:.*]] = !DIBasicType(name: "integer", size: 64, encoding: DW_ATE_signed)
 // CHECK-DAG: ![[SCOPE:.*]] = distinct !DICompileUnit(language: DW_LANG_Fortran95, file: ![[FILE]], producer: "MLIR", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, globals: ![[GVALS:.*]])

mlir/test/Target/LLVMIR/llvmir.mlir

@@ -2774,11 +2774,22 @@ module {
                      #llvm.mlir.module_flag<max, "frame-pointer", 1>]
 }
 
-// CHECK: !llvm.module.flags = !{![[#DBG:]], ![[#WCHAR:]], ![[#PIC:]], ![[#PIE:]], ![[#UWTABLE:]], ![[#FrameP:]]}
+// CHECK: !llvm.module.flags = !{![[#WCHAR:]], ![[#PIC:]], ![[#PIE:]], ![[#UWTABLE:]], ![[#FrameP:]], ![[#DBG:]]}
 
-// CHECK: ![[#DBG]] = !{i32 2, !"Debug Info Version", i32 3}
 // CHECK: ![[#WCHAR]] = !{i32 1, !"wchar_size", i32 4}
 // CHECK: ![[#PIC]] = !{i32 8, !"PIC Level", i32 2}
 // CHECK: ![[#PIE]] = !{i32 7, !"PIE Level", i32 2}
 // CHECK: ![[#UWTABLE]] = !{i32 7, !"uwtable", i32 2}
 // CHECK: ![[#FrameP]] = !{i32 7, !"frame-pointer", i32 1}
+// CHECK: ![[#DBG]] = !{i32 2, !"Debug Info Version", i32 3}
+
+// -----
+
+// Verifies that the debug info version is not added twice, if it's already present initially.
+
+module {
+  llvm.module_flags [#llvm.mlir.module_flag<warning, "Debug Info Version", 3>]
+}
+
+// CHECK: !llvm.module.flags = !{![[#DBG:]]}
+// CHECK: ![[#DBG]] = !{i32 2, !"Debug Info Version", i32 3}

mlir/test/Target/LLVMIR/nvvmir.mlir

@@ -646,17 +646,17 @@ llvm.func @kernel_func() attributes {nvvm.kernel, nvvm.maxntid = array<i32: 1, 2
 // -----
 // CHECK: define ptx_kernel void @kernel_func
 // CHECK: !nvvm.annotations =
-// CHECK: !1 = !{ptr @kernel_func, !"grid_constant", !2}
-// CHECK: !2 = !{i32 1}
+// CHECK: !{{.*}} = !{ptr @kernel_func, !"grid_constant", ![[ID:[[:alnum:]]+]]}
+// CHECK: ![[ID]] = !{i32 1}
 llvm.func @kernel_func(%arg0: !llvm.ptr {llvm.byval = i32, nvvm.grid_constant}) attributes {nvvm.kernel} {
   llvm.return
 }
 
 // -----
 // CHECK: define ptx_kernel void @kernel_func
 // CHECK: !nvvm.annotations =
-// CHECK: !1 = !{ptr @kernel_func, !"grid_constant", !2}
-// CHECK: !2 = !{i32 1, i32 3}
+// CHECK: !{{.*}} = !{ptr @kernel_func, !"grid_constant", ![[ID:[[:alnum:]]+]]}
+// CHECK: ![[ID]] = !{i32 1, i32 3}
 llvm.func @kernel_func(%arg0: !llvm.ptr {llvm.byval = i32, nvvm.grid_constant}, %arg1: f32, %arg2: !llvm.ptr {llvm.byval = f32, nvvm.grid_constant}) attributes {nvvm.kernel} {
   llvm.return
 }