[mir-strip-debug] Fix debug location info strip for bundled instructions

[ivafanas]

Fix bug that mir-strip-debug pass does not remove debug location from bundled instructions.

Problem arises during testing that debug info does not affect optimization passes output (llvm-lit with -Dllc="llc -debugify-and-strip-all-safe"), when pass operates on MIR with bundled instructions + memory operands.

Let mir test check looks like:

CHECK-NEXT: BUNDLE {
CHECK-NEXT:   $r3 = LD $r1, $r2 :: (load (s64) from %ir.a, !tbaa !2)
CHECK-NEXT: }

So as mir-strip-debug pass does not process bundled instructions, running llc -debugify-and-strip-all-safe on the test will produce the following output:

BUNDLE {
  $r3 = LD $r1, $r2, debug-location !DILocation(line: 3, column: 1, scope: <0x608cb2b99b10>) :: (load (s64) from %ir.a, !tbaa !2)
}

And test will fail, but it shouldn't.

Seems like the root cause is that mir-strip-debug pass should remove debug location from bundled instructions.

[llvmbot]

@llvm/pr-subscribers-debuginfo

Author: Afanasyev Ivan (ivafanas)

Changes

Fix bug that mir-strip-debug pass does not remove debug location from bundled instructions.

Problem arises during testing that debug info does not affect optimization passes output (llvm-lit with -Dllc="llc -debugify-and-strip-all-safe"), when pass operates on MIR with bundled instructions + memory operands.

Let mir test check looks like:

CHECK-NEXT: BUNDLE {
CHECK-NEXT:   $r3 = LD $r1, $r2 :: (load (s64) from %ir.a, !tbaa !2)
CHECK-NEXT: }

So as mir-strip-debug pass does not process bundled instructions, running llc -debugify-and-strip-all-safe on the test will produce the following output:

BUNDLE {
  $r3 = LD $r1, $r2, debug-location !DILocation(line: 3, column: 1, scope: <0x608cb2b99b10>) :: (load (s64) from %ir.a, !tbaa !2)
}

And test will fail, but it shouldn't.

Seems like the root cause is that mir-strip-debug pass should remove debug location from bundled instructions.

---

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

2 Files Affected:

• (modified) llvm/lib/CodeGen/MachineStripDebug.cpp (+1-1)
• (added) llvm/test/CodeGen/Generic/MIRStripDebug/bundles.mir (+63)

diff --git a/llvm/lib/CodeGen/MachineStripDebug.cpp b/llvm/lib/CodeGen/MachineStripDebug.cpp
index 6128248a028e3e..ea291f64bff432 100644
--- a/llvm/lib/CodeGen/MachineStripDebug.cpp
+++ b/llvm/lib/CodeGen/MachineStripDebug.cpp
@@ -50,7 +50,7 @@ struct StripDebugMachineModule : public ModulePass {
         continue;
       MachineFunction &MF = *MaybeMF;
       for (MachineBasicBlock &MBB : MF) {
-        for (MachineInstr &MI : llvm::make_early_inc_range(MBB)) {
+        for (MachineInstr &MI : llvm::make_early_inc_range(MBB.instrs())) {
           if (MI.isDebugInstr()) {
             // FIXME: We should remove all of them. However, AArch64 emits an
             //        invalid `DBG_VALUE $lr` with only one operand instead of
diff --git a/llvm/test/CodeGen/Generic/MIRStripDebug/bundles.mir b/llvm/test/CodeGen/Generic/MIRStripDebug/bundles.mir
new file mode 100644
index 00000000000000..111c886f585cf6
--- /dev/null
+++ b/llvm/test/CodeGen/Generic/MIRStripDebug/bundles.mir
@@ -0,0 +1,63 @@
+# RUN: llc -run-pass=mir-strip-debug -o - %s | FileCheck %s
+# RUN: llc -run-pass=mir-strip-debug,mir-debugify,mir-strip-debug -o - %s | FileCheck %s
+
+--- |
+  source_filename = "loc-only.ll"
+  
+  define i32 @test(i32 %a, i32 %b) !dbg !6 {
+    %add = add i32 %a, 2, !dbg !12
+    call void @llvm.dbg.value(metadata i32 %add, metadata !9, metadata !DIExpression()), !dbg !12
+    %sub = sub i32 %add, %b, !dbg !13
+    call void @llvm.dbg.value(metadata i32 %sub, metadata !11, metadata !DIExpression()), !dbg !13
+    ret i32 %sub, !dbg !14
+  }
+  
+  declare void @llvm.dbg.value(metadata, metadata, metadata)
+  
+  !llvm.dbg.cu = !{!0}
+  ; CHECK-NOT: !llvm.dbg.cu
+  !llvm.debugify = !{!3, !4}
+  ; CHECK-NOT: !llvm.debugify
+  !llvm.module.flags = !{!5}
+  ; CHECK-NOT: !llvm.module.flags
+
+  ; CHECK-NOT: !DI
+  !0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+  !1 = !DIFile(filename: "<stdin>", directory: "/")
+  !2 = !{}
+  !3 = !{i32 3}
+  !4 = !{i32 2}
+  !5 = !{i32 2, !"Debug Info Version", i32 3}
+  !6 = distinct !DISubprogram(name: "test", linkageName: "test", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
+  !7 = !DISubroutineType(types: !2)
+  !8 = !{!9, !11}
+  !9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10)
+  !10 = !DIBasicType(name: "ty32", size: 32, encoding: DW_ATE_unsigned)
+  !11 = !DILocalVariable(name: "2", scope: !6, file: !1, line: 2, type: !10)
+  !12 = !DILocation(line: 1, column: 1, scope: !6)
+  !13 = !DILocation(line: 2, column: 1, scope: !6)
+  !14 = !DILocation(line: 3, column: 1, scope: !6)
+
+...
+---
+name:            test
+body:             |
+  bb.1 (%ir-block.0):
+    %0:_(s32) = G_IMPLICIT_DEF
+    %1:_(s32) = G_IMPLICIT_DEF
+    BUNDLE {
+      %2:_(s32) = G_CONSTANT i32 2, debug-location !DILocation(line: 0, scope: !6)
+      %3:_(s32) = G_ADD %0, %1, debug-location !12
+    }
+
+    ; CHECK-LABEL: body:
+    ; CHECK-NOT: debug-location
+    ; CHECK-NOT: !DI
+    ; CHECK-NEXT:    bb
+    ; CHECK-NEXT:      %0:_(s32) = G_IMPLICIT_DEF{{$}}
+    ; CHECK-NEXT:      %1:_(s32) = G_IMPLICIT_DEF{{$}}
+    ; CHECK-NEXT:      BUNDLE {
+    ; CHECK-NEXT:        %2:_(s32) = G_CONSTANT i32 2{{$}}
+    ; CHECK-NEXT:        %3:_(s32) = G_ADD %0, %1{{$}}
+    ; CHECK-NEXT:      }
+...

[ivafanas]

@dsandersllvm @echristo

Could you please review the PR?

[dsandersllvm]

LGTM, thanks

Would you be willing to (in a different PR) add doxygen comments to .instrs() highlighting the difference that .instrs() makes for bundles compared to just iterating over the block?

[ivafanas]

Thank you!

Yes, I'll add comment.

[ivafanas]

Hi, @dsandersllvm.

If you have write access to the repository, could you please merge the PR?
I do not have write access.

[ivafanas]

Thank you!

Yes, I'll add comment.

Done:
#113868