Skip to content

[MLIR][LLVM] Avoid exporting broken debug intrinsics without a location #70643

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 30, 2023

Conversation

Dinistro
Copy link
Contributor

LLVM IR does not allow debug intrinsics without a debug attachment. The location export can fail the export of a location due to multiple reasons. To deal with this, this commit adds a check to the debug intrinsic's LLVM builders, that skips them, if the location is nullptr.

Fixes #60222

LLVM IR does not allow debug intrinsics without a debug attachment. The
location export can fail the export of a location due to multiple
reasons. To deal with this, this commit adds a check to the debug intrinsic's
LLVM builders, that skips them, if the location is `nullptr`.
@llvmbot
Copy link
Member

llvmbot commented Oct 30, 2023

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-llvm

Author: Christian Ulmann (Dinistro)

Changes

LLVM IR does not allow debug intrinsics without a debug attachment. The location export can fail the export of a location due to multiple reasons. To deal with this, this commit adds a check to the debug intrinsic's LLVM builders, that skips them, if the location is nullptr.

Fixes #60222


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

2 Files Affected:

  • (modified) mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td (+6)
  • (modified) mlir/test/Target/LLVMIR/llvmir-debug.mlir (+31)
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td
index 040f9895ad0dba6..72c932ac07a2e1a 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td
@@ -526,6 +526,9 @@ def LLVM_CoroResumeOp : LLVM_IntrOp<"coro.resume", [], [], [], 0> {
 class LLVM_DbgIntrOp<string name, string argName, list<Trait> traits = []>
     : LLVM_IntrOp<name, [], [], traits, 0> {
   let llvmBuilder = [{
+    // Debug intrinsics without debug locations are invalid.
+    if(!builder.getCurrentDebugLocation())
+      return success();
     llvm::Module *module = builder.GetInsertBlock()->getModule();
     llvm::LLVMContext &ctx = module->getContext();
     llvm::Function *fn =
@@ -566,6 +569,9 @@ def LLVM_DbgLabelOp : LLVM_IntrOp<"dbg.label", [], [], [], 0> {
   let summary = "Relates the program to a debug information label.";
   let arguments = (ins LLVM_DILabelAttr:$label);
   let llvmBuilder = [{
+    // Debug intrinsics without debug locations are invalid.
+    if(!builder.getCurrentDebugLocation())
+      return success();
     llvm::Module *module = builder.GetInsertBlock()->getModule();
     llvm::LLVMContext &ctx = module->getContext();
     llvm::Function *fn =
diff --git a/mlir/test/Target/LLVMIR/llvmir-debug.mlir b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
index c1e3d723df6675a..8d1734d7cdc3117 100644
--- a/mlir/test/Target/LLVMIR/llvmir-debug.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
@@ -232,3 +232,34 @@ llvm.func @func_without_subprogram(%0 : i32) {
 // 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]]
+
+// -----
+
+// Ensures that debug intrinsics without a valid location are not exported to
+// avoid broken LLVM IR.
+
+#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 = "outer_func",
+  file = #di_file, subprogramFlags = "Definition|Optimized"
+>
+#di_local_variable = #llvm.di_local_variable<scope = #di_subprogram, name = "a">
+#declared_var = #llvm.di_local_variable<scope = #di_subprogram, name = "alloc">
+#di_label = #llvm.di_label<scope = #di_subprogram, name = "label", file = #di_file, line = 42>
+
+// CHECK-LABEL: define i32 @dbg_intrinsics_with_no_location(
+llvm.func @dbg_intrinsics_with_no_location(%arg0: i32) -> (i32) {
+  %allocCount = llvm.mlir.constant(1 : i32) : i32
+  %alloc = llvm.alloca %allocCount x i64 : (i32) -> !llvm.ptr
+  // CHECK-NOT: @llvm.dbg.value
+  llvm.intr.dbg.value #di_local_variable = %arg0 : i32
+  // CHECK-NOT: @llvm.dbg.declare
+  llvm.intr.dbg.declare #declared_var = %alloc : !llvm.ptr
+  // CHECK-NOT: @llvm.dbg.label
+  llvm.intr.dbg.label #di_label
+  llvm.return %arg0 : i32
+}

Copy link
Contributor

@gysit gysit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Dinistro Dinistro merged commit 3e96070 into llvm:main Oct 30, 2023
@Dinistro Dinistro deleted the stop-exporting-broken-dbg-intrinsics branch October 30, 2023 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[mlir] MLIR -> LLVM IR debug location mapping fails on name-only locations
3 participants