[RemoveDIs] Resolve RemoveRedundantDbgInstrs fwd scan FIXME

[OCHyams]

These FIXMEs were added to keep the dbg_record implementation identical to the dbg intrinsic versions, which have since been removed. I don't think there's any reason for the old behaviour; my understanding is it was a minor bug no one got round to fixing.

I've upgraded the test to be written with dbg_records while I'm here.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-debuginfo

Author: Orlando Cazalet-Hyams (OCHyams)

Changes

These FIXMEs were added to keep the dbg_record implementation identical to the dbg intrinsic versions, which have since been removed. I don't think there's any reason for the old behaviour; my understanding is it was a minor bug no one got round to fixing.

I've upgraded the test to be written with dbg_records while I'm here.

---

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

2 Files Affected:

• (modified) llvm/lib/Transforms/Utils/BasicBlockUtils.cpp (+2-21)
• (modified) llvm/test/Transforms/DCE/dbg-value-removal.ll (+31-29)

diff --git a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
index 98c65ae11b1c3..c8255742c41ba 100644
--- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -377,33 +377,14 @@ bool llvm::MergeBlockSuccessorsIntoGivenBlocks(
 ///
 /// Possible improvements:
 /// - Check fully overlapping fragments and not only identical fragments.
-/// - Support dbg.declare. dbg.label, and possibly other meta instructions being
-///   part of the sequence of consecutive instructions.
 static bool
 DbgVariableRecordsRemoveRedundantDbgInstrsUsingBackwardScan(BasicBlock *BB) {
   SmallVector<DbgVariableRecord *, 8> ToBeRemoved;
   SmallDenseSet<DebugVariable> VariableSet;
   for (auto &I : reverse(*BB)) {
-    for (DbgRecord &DR : reverse(I.getDbgRecordRange())) {
-      if (isa<DbgLabelRecord>(DR)) {
-        // Emulate existing behaviour (see comment below for dbg.declares).
-        // FIXME: Don't do this.
-        VariableSet.clear();
-        continue;
-      }
-
+    for (DbgVariableRecord &DR :
+         reverse(filterDbgVars(I.getDbgRecordRange()))) {
       DbgVariableRecord &DVR = cast<DbgVariableRecord>(DR);
-      // Skip declare-type records, as the debug intrinsic method only works
-      // on dbg.value intrinsics.
-      if (DVR.getType() == DbgVariableRecord::LocationType::Declare) {
-        // The debug intrinsic method treats dbg.declares are "non-debug"
-        // instructions (i.e., a break in a consecutive range of debug
-        // intrinsics). Emulate that to create identical outputs. See
-        // "Possible improvements" above.
-        // FIXME: Delete the line below.
-        VariableSet.clear();
-        continue;
-      }
 
       DebugVariable Key(DVR.getVariable(), DVR.getExpression(),
                         DVR.getDebugLoc()->getInlinedAt());
diff --git a/llvm/test/Transforms/DCE/dbg-value-removal.ll b/llvm/test/Transforms/DCE/dbg-value-removal.ll
index 724ad7ba3a092..ec850ac77bed4 100644
--- a/llvm/test/Transforms/DCE/dbg-value-removal.ll
+++ b/llvm/test/Transforms/DCE/dbg-value-removal.ll
@@ -4,7 +4,7 @@
 ; All dbg.value with location "!dbg !19" are redundant in the input.
 ; FIXME: We do not handle non-overlapping/overlapping fragments perfectly yet.
 
-define dso_local i16 @main(i16 %a1, i16 %a2) local_unnamed_addr #0 !dbg !7 {
+define dso_local i16 @main(i16 %a1, i16 %a2) local_unnamed_addr !dbg !7 {
 ; CHECK-LABEL: @main(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    br label [[BB0:%.*]]
@@ -23,6 +23,8 @@ define dso_local i16 @main(i16 %a1, i16 %a2) local_unnamed_addr #0 !dbg !7 {
 ; CHECK-NEXT:      #dbg_value(i16 [[A2]], [[META12]], !DIExpression(DW_OP_constu, 2, DW_OP_shr, DW_OP_stack_value), [[META18]])
 ; CHECK-NEXT:    br label [[BB2:%.*]]
 ; CHECK:       bb2:
+; CHECK-NEXT:      #dbg_declare(i16 %a1, !19, !DIExpression(), [[META18]])
+; CHECK-NEXT:      #dbg_label([[META21:![0-9]+]], [[META18]])
 ; CHECK-NEXT:      #dbg_value(i16 [[A1]], [[META13]], !DIExpression(DW_OP_LLVM_fragment, 0, 8), [[META18]])
 ; CHECK-NEXT:      #dbg_value(i16 [[A1]], [[META13]], !DIExpression(DW_OP_LLVM_fragment, 8, 8), [[META18]])
 ; CHECK-NEXT:    [[T2:%.*]] = call i16 @bar(i16 [[T1]])
@@ -40,51 +42,49 @@ entry:
   br label %bb0
 
 bb0:
-  call void @llvm.dbg.value(metadata i16 999, metadata !12, metadata !DIExpression()), !dbg !19
-  call void @llvm.dbg.value(metadata i16 996, metadata !13, metadata !DIExpression()), !dbg !19
-  call void @llvm.dbg.value(metadata i16 13, metadata !13, metadata !DIExpression()), !dbg !17
-  call void @llvm.dbg.value(metadata i16 998, metadata !12, metadata !DIExpression(DW_OP_constu, 2, DW_OP_shr, DW_OP_stack_value)), !dbg !19
-  call void @llvm.dbg.value(metadata i16 14, metadata !14, metadata !DIExpression()), !dbg !16
-  call void @llvm.dbg.value(metadata i16 997, metadata !12, metadata !DIExpression()), !dbg !19
-  call void @llvm.dbg.value(metadata i16 13, metadata !13, metadata !DIExpression()), !dbg !16
-  call void @llvm.dbg.value(metadata i16 12, metadata !12, metadata !DIExpression()), !dbg !16
+    #dbg_value(i16 999, !12, !DIExpression(), !19)
+    #dbg_value(i16 996, !13, !DIExpression(), !19)
+    #dbg_value(i16 13, !13, !DIExpression(), !17)
+    #dbg_value(i16 998, !12, !DIExpression(DW_OP_constu, 2, DW_OP_shr, DW_OP_stack_value), !19)
+    #dbg_value(i16 14, !14, !DIExpression(), !16)
+    #dbg_value(i16 997, !12, !DIExpression(), !19)
+    #dbg_value(i16 13, !13, !DIExpression(), !16)
+    #dbg_value(i16 12, !12, !DIExpression(), !16)
   br label %bb1
 
 bb1:
-  call void @llvm.dbg.value(metadata i16 %a1, metadata !14, metadata !DIExpression()), !dbg !16
-  call void @llvm.dbg.value(metadata i16 888, metadata !13, metadata !DIExpression()), !dbg !16
-  call void @llvm.dbg.value(metadata i16 %a2, metadata !12, metadata !DIExpression()), !dbg !16
+    #dbg_value(i16 %a1, !14, !DIExpression(), !16)
+    #dbg_value(i16 888, !13, !DIExpression(), !16)
+    #dbg_value(i16 %a2, !12, !DIExpression(), !16)
   %t1 = call i16 @bar(i16 0)
-  call void @llvm.dbg.value(metadata i16 %a1, metadata !14, metadata !DIExpression()), !dbg !19
-  call void @llvm.dbg.value(metadata i16 %t1, metadata !13, metadata !DIExpression()), !dbg !16
-  call void @llvm.dbg.value(metadata i16 %a2, metadata !12, metadata !DIExpression(DW_OP_constu, 2, DW_OP_shr, DW_OP_stack_value)), !dbg !16
+    #dbg_value(i16 %a1, !14, !DIExpression(), !19)
+    #dbg_value(i16 %t1, !13, !DIExpression(), !16)
+    #dbg_value(i16 %a2, !12, !DIExpression(DW_OP_constu, 2, DW_OP_shr, DW_OP_stack_value), !16)
   br label %bb2
 
 bb2:
-  call void @llvm.dbg.value(metadata i16 %a1, metadata !13, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 8)), !dbg !19
-  call void @llvm.dbg.value(metadata i16 %a1, metadata !13, metadata !DIExpression(DW_OP_LLVM_fragment, 8, 8)), !dbg !19
-  call void @llvm.dbg.value(metadata i16 %a1, metadata !13, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 8)), !dbg !16
-  call void @llvm.dbg.value(metadata i16 %a1, metadata !13, metadata !DIExpression(DW_OP_LLVM_fragment, 8, 8)), !dbg !16
+    #dbg_value(i16 %a1, !13, !DIExpression(DW_OP_LLVM_fragment, 0, 8), !19)
+    #dbg_value(i16 %a1, !13, !DIExpression(DW_OP_LLVM_fragment, 8, 8), !19)
+    ; Check dbg_declare and dbg_label don't interfere with the backward scan.
+    #dbg_declare(i16 %a1, !21, !DIExpression(), !16)
+    #dbg_label(!20, !16)
+    #dbg_value(i16 %a1, !13, !DIExpression(DW_OP_LLVM_fragment, 0, 8), !16)
+    #dbg_value(i16 %a1, !13, !DIExpression(DW_OP_LLVM_fragment, 8, 8), !16)
   %t2 = call i16 @bar(i16 %t1)
-  call void @llvm.dbg.value(metadata i16 %t2, metadata !13, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 8)), !dbg !16
-  call void @llvm.dbg.value(metadata i16 %a1, metadata !13, metadata !DIExpression(DW_OP_LLVM_fragment, 8, 8)), !dbg !19
+    #dbg_value(i16 %t2, !13, !DIExpression(DW_OP_LLVM_fragment, 0, 8), !16)
+    #dbg_value(i16 %a1, !13, !DIExpression(DW_OP_LLVM_fragment, 8, 8), !19)
   br label %bb3
 
 bb3:
-  call void @llvm.dbg.value(metadata i16 %a1, metadata !13, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 8)), !dbg !19
-  call void @llvm.dbg.value(metadata i16 %a1, metadata !13, metadata !DIExpression()), !dbg !16
+    #dbg_value(i16 %a1, !13, !DIExpression(DW_OP_LLVM_fragment, 0, 8), !19)
+    #dbg_value(i16 %a1, !13, !DIExpression(), !16)
   br label %exit
 
 exit:
   ret i16 %t2
 }
 
-declare void @llvm.dbg.value(metadata, metadata, metadata) #1
-declare i16 @bar(i16) #2
-
-attributes #0 = { noinline nounwind }
-attributes #1 = { nounwind readnone speculatable willreturn }
-attributes #2 = { noinline nounwind readnone }
+declare i16 @bar(i16)
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!3, !4, !5}
@@ -110,3 +110,5 @@ attributes #2 = { noinline nounwind readnone }
 !17 = !DILocation(line: 0, scope: !7, inlinedAt: !18)
 !18 = !DILocation(line: 1, scope: !7)
 !19 = !DILocation(line: 77, scope: !7)
+!20 = !DILabel(scope: !7, name: "label", file: !1, line: 3)
+!21 = !DILocalVariable(name: "z", scope: !7, file: !1, line: 10, type: !10)