[Mlir-commits] [mlir] a902ca6 - [MLIR][LLVM] Infer export location scope from location, if possible (#70465)

[llvmlistbot at llvm.org]

Author: Christian Ulmann
Date: 2023-10-30T10:42:04+01:00
New Revision: a902ca66428164b4f11dedf1c551393add511271

URL: https://github.com/llvm/llvm-project/commit/a902ca66428164b4f11dedf1c551393add511271
DIFF: https://github.com/llvm/llvm-project/commit/a902ca66428164b4f11dedf1c551393add511271.diff

LOG: [MLIR][LLVM] Infer export location scope from location, if possible (#70465)

This commit changes the debug location exporter to try to infer the
locations scope from the MLIR location, if possible. This is necessary
when the function containing the operation does not have a DISubprogram
attached to it.

We observed a roundtrip crash with a case where the the subprogram was
missing on a function, but a debug intrinsic referenced a subprogram
non-the-less. This lead to a successful import, but the export silently
dropped the location, which results in invalid IR.

Added: 
    

Modified: 
    mlir/lib/Target/LLVMIR/DebugTranslation.cpp
    mlir/test/Target/LLVMIR/llvmir-debug.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
index 52a01d7e8e0e1d0..a15ffb40065a08b 100644
--- a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
@@ -286,7 +286,7 @@ llvm::DILocation *DebugTranslation::translateLoc(Location loc,
                                                  llvm::DILocalScope *scope,
                                                  llvm::DILocation *inlinedAt) {
   // LLVM doesn't have a representation for unknown.
-  if (!scope || isa<UnknownLoc>(loc))
+  if (isa<UnknownLoc>(loc))
     return nullptr;
 
   // Check for a cached instance.
@@ -301,17 +301,12 @@ llvm::DILocation *DebugTranslation::translateLoc(Location loc,
     llvmLoc = translateLoc(callLoc.getCallee(), scope, callerLoc);
 
   } else if (auto fileLoc = dyn_cast<FileLineColLoc>(loc)) {
-    llvm::DILocalScope *locationScope = scope;
-    // Only construct a new DIFile when no local scope is present. This
-    // prioritizes existing DI information when it's present.
-    if (!locationScope) {
-      auto *file = translateFile(fileLoc.getFilename());
-      locationScope = llvm::DILexicalBlockFile::get(llvmCtx, scope, file,
-                                                    /*Discriminator=*/0);
-    }
-    llvmLoc = llvm::DILocation::get(llvmCtx, fileLoc.getLine(),
-                                    fileLoc.getColumn(), locationScope,
-                                    const_cast<llvm::DILocation *>(inlinedAt));
+    // A scope of a DILocation cannot be null.
+    if (!scope)
+      return nullptr;
+    llvmLoc =
+        llvm::DILocation::get(llvmCtx, fileLoc.getLine(), fileLoc.getColumn(),
+                              scope, const_cast<llvm::DILocation *>(inlinedAt));
 
   } else if (auto fusedLoc = dyn_cast<FusedLoc>(loc)) {
     ArrayRef<Location> locations = fusedLoc.getLocations();

diff  --git a/mlir/test/Target/LLVMIR/llvmir-debug.mlir b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
index 9156c3a1bbbb490..c1e3d723df6675a 100644
--- a/mlir/test/Target/LLVMIR/llvmir-debug.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
@@ -205,3 +205,30 @@ llvm.func @func_with_inlined_dbg_value(%arg0: i32) -> (i32) {
 // CHECK-DAG: ![[VAR_LOC0]] = !DILocalVariable(name: "a", scope: ![[OUTER_FUNC]], file: ![[FILE]]
 // CHECK-DAG: ![[VAR_LOC1]] = !DILocalVariable(name: "b", scope: ![[LEXICAL_BLOCK_FILE]], file: ![[FILE]]
 // CHECK-DAG  ![[LABEL]] = !DILabel(scope: ![[LEXICAL_BLOCK_FILE]], name: "label", file: ![[FILE]], line: 42)
+
+// -----
+
+#di_basic_type = #llvm.di_basic_type<tag = DW_TAG_base_type, name = "int", sizeInBits = 32, encoding = DW_ATE_signed>
+#di_file = #llvm.di_file<"foo.mlir" in "/test/">
+#di_compile_unit = #llvm.di_compile_unit<
+  sourceLanguage = DW_LANG_C, file = #di_file, producer = "MLIR",
+  isOptimized = true, emissionKind = Full
+>
+#di_subprogram = #llvm.di_subprogram<
+  compileUnit = #di_compile_unit, scope = #di_file, name = "func",
+  file = #di_file, subprogramFlags = Definition>
+#di_local_variable = #llvm.di_local_variable<scope = #di_subprogram, name = "a", file = #di_file, type = #di_basic_type>
+
+#loc = loc("foo.mlir":0:0)
+
+// CHECK-LABEL: define void @func_without_subprogram(
+// CHECK-SAME: i32 %[[ARG:.*]])
+llvm.func @func_without_subprogram(%0 : i32) {
+  // CHECK: call void @llvm.dbg.value(metadata i32 %[[ARG]], metadata ![[VAR_LOC:[0-9]+]], metadata !DIExpression()), !dbg ![[DBG_LOC0:.*]]
+  llvm.intr.dbg.value #di_local_variable = %0 : i32 loc(fused<#di_subprogram>[#loc])
+  llvm.return
+}
+
+// CHECK: ![[FILE:.*]] = !DIFile(filename: "foo.mlir", directory: "/test/")
+// CHECK-DAG: ![[FUNC:.*]] = distinct !DISubprogram(name: "func", scope: ![[FILE]]
+// CHECK-DAG: ![[VAR_LOC]] = !DILocalVariable(name: "a", scope: ![[FUNC]], file: ![[FILE]]