[Mlir-commits] [mlir] c04d08c - [mlir][llvm] Dominance violating debug intrinsic import

[Christian Ulmann]

Author: Christian Ulmann
Date: 2023-06-29T08:14:09Z
New Revision: c04d08c24fa06cbf9531ddd5140a4fe9e0d4b678

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

LOG: [mlir][llvm] Dominance violating debug intrinsic import

Debug intrinsics are allowed to violate SSA dominance and might thus
cause the LLVM import to produce invalid LLVM dialect. This commit
ensures that the debug intrinsics are emitted right after the definition
of their SSA operands.
As the position of debug intrinsics has no meaning, changing it has no
semantic implication.

Reviewed By: gysit

Differential Revision: https://reviews.llvm.org/D153984

Added: 
    

Modified: 
    mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td
    mlir/test/Target/LLVMIR/Import/debug-info.ll

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td
index d673ec1cd97131..e8065daaf5da98 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td
@@ -546,8 +546,16 @@ class LLVM_DbgIntrOp<string name, string argName, list<Trait> traits = []>
     // Drop the intrinsic when its operand could not be converted. This can
     // happen for use before definition cases that are allowed for debug
     // intrinsics.
+    // TODO: Implement a two pass solution that translates the debug intrinsics
+    // after the entire function as been translated.
     if (failed(argOperand))
       return success();
+
+    // Ensure that the debug instrinsic is inserted right after its operand is
+    // defined. Otherwise, the operand might not necessarily dominate the
+    // intrinsic.
+    OpBuilder::InsertionGuard guard($_builder);
+    $_builder.setInsertionPointAfterValue(*argOperand);
     $_op = $_builder.create<$_qualCppClassName>($_location,
         *argOperand, $_var_attr($varInfo));
   }];

diff  --git a/mlir/test/Target/LLVMIR/Import/debug-info.ll b/mlir/test/Target/LLVMIR/Import/debug-info.ll
index 01eb7638c23feb..d39873b6d0d526 100644
--- a/mlir/test/Target/LLVMIR/Import/debug-info.ll
+++ b/mlir/test/Target/LLVMIR/Import/debug-info.ll
@@ -233,27 +233,30 @@ source_filename = "debug-info.ll"
 
 ; // -----
 
+; NOTE: The debug intrinsics are reordered as a side-effect of the dominance-
+;       preserving measures needed to import LLVM IR.
+
 ; CHECK: #[[FILE:.+]] = #llvm.di_file<
 ; CHECK: #[[$SP:.+]] = #llvm.di_subprogram<
 ; CHECK: #[[$LABEL:.+]] = #llvm.di_label<scope = #[[$SP]], name = "label", file = #[[FILE]], line = 42>
-; CHECK: #[[$VAR0:.+]] = #llvm.di_local_variable<scope = #[[$SP]], name = "arg", file = #[[FILE]], line = 1, arg = 1, alignInBits = 32, type = #{{.*}}>
 ; CHECK: #[[$VAR1:.+]] = #llvm.di_local_variable<scope = #[[$SP]], name = "arg">
+; CHECK: #[[$VAR0:.+]] = #llvm.di_local_variable<scope = #[[$SP]], name = "arg", file = #[[FILE]], line = 1, arg = 1, alignInBits = 32, type = #{{.*}}>
 
 ; CHECK-LABEL: @intrinsic
 ; CHECK-SAME:  %[[ARG0:[a-zA-Z0-9]+]]
 ; CHECK-SAME:  %[[ARG1:[a-zA-Z0-9]+]]
 define void @intrinsic(i64 %0, ptr %1) {
+  ; CHECK: llvm.intr.dbg.declare #[[$VAR1]] = %[[ARG1]] : !llvm.ptr loc(#[[LOC1:.+]])
   ; CHECK: llvm.intr.dbg.value #[[$VAR0]] = %[[ARG0]] : i64 loc(#[[LOC0:.+]])
   call void @llvm.dbg.value(metadata i64 %0, metadata !5, metadata !DIExpression()), !dbg !7
-  ; CHECK: llvm.intr.dbg.declare #[[$VAR1]] = %[[ARG1]] : !llvm.ptr loc(#[[LOC1:.+]])
   call void @llvm.dbg.declare(metadata ptr %1, metadata !6, metadata !DIExpression()), !dbg !9
   ; CHECK: llvm.intr.dbg.label #[[$LABEL]] loc(#[[LOC1:.+]])
   call void @llvm.dbg.label(metadata !10), !dbg !9
   ret void
 }
 
-; CHECK: #[[LOC0]] = loc(fused<#[[$SP]]>[{{.*}}])
 ; CHECK: #[[LOC1]] = loc(fused<#[[$SP]]>[{{.*}}])
+; CHECK: #[[LOC0]] = loc(fused<#[[$SP]]>[{{.*}}])
 
 declare void @llvm.dbg.value(metadata, metadata, metadata)
 declare void @llvm.dbg.declare(metadata, metadata, metadata)
@@ -372,6 +375,40 @@ declare void @llvm.dbg.value(metadata, metadata, metadata)
 
 ; // -----
 
+; This test checks that broken dominance doesn't break the metadata import.
+
+; CHECK-LABEL: @dbg_broken_dominance
+define void @dbg_broken_dominance(ptr %arg, i1 %cond) {
+  br i1 %cond, label %b1, label %b2
+b1:
+  br label %b3
+b2:
+  %dbg_arg = getelementptr double, ptr %arg, i64 16
+  ; CHECK: llvm.getelementptr
+  ; CHECK-NEXT: llvm.intr.dbg.value
+  br label %b3
+b3:
+  call void @llvm.dbg.value(metadata ptr %dbg_arg, metadata !7, metadata !DIExpression()), !dbg !9
+  ret void
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!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 = !DICompositeType(tag: DW_TAG_class_type, name: "class_field", file: !2, line: 42, flags: DIFlagTypePassByReference | DIFlagNonTrivial, elements: !4)
+!4 = !{!6}
+!5 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !3, flags: DIFlagArtificial | DIFlagObjectPointer)
+!6 = !DIDerivedType(tag: DW_TAG_member, name: "call_field", file: !2, baseType: !5)
+!7 = !DILocalVariable(scope: !8, name: "var", file: !2, type: !5);
+!8 = distinct !DISubprogram(name: "dbg_use_before_def", scope: !2, file: !2, spFlags: DISPFlagDefinition, unit: !1)
+!9 = !DILocation(line: 1, column: 2, scope: !8)
+
+; // -----
+
 ; CHECK-DAG: #[[NAMESPACE:.+]] = #llvm.di_namespace<name = "std", exportSymbols = false>
 ; CHECK-DAG: #[[SUBPROGRAM:.+]] =  #llvm.di_subprogram<compileUnit = #{{.*}}, scope = #[[NAMESPACE]], name = "namespace"