[RemoveDIs] Fix SimplifyCFG behaviour to match existing behaviour

[OCHyams]

llvm.dbg.labels are deleted in SpeculativelyExecuteBB so DPLabels should be too.

Modify existing test to check this (NB I couldn't find a dedicated debug-info test that checks this behaviour).

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Orlando Cazalet-Hyams (OCHyams)

Changes

llvm.dbg.labels are deleted in SpeculativelyExecuteBB so DPLabels should be too.

Modify existing test to check this (NB I couldn't find a dedicated debug-info test that checks this behaviour).

---

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

2 Files Affected:

• (modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (+2-2)
• (modified) llvm/test/Transforms/SimplifyCFG/branch-fold-dbg.ll (+7-1)

diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 9562d44400448b..df29d445e2a651 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -3208,8 +3208,8 @@ bool SimplifyCFGOpt::SpeculativelyExecuteBB(BranchInst *BI,
   // end of this block.
   for (auto &It : make_range(ThenBB->begin(), ThenBB->end()))
     for (DbgRecord &DR : make_early_inc_range(It.getDbgValueRange()))
-      if (DPValue *DPV = dyn_cast<DPValue>(&DR); DPV && !DPV->isDbgAssign())
-        It.dropOneDbgValue(DPV);
+      if (DPValue *DPV = dyn_cast<DPValue>(&DR); !DPV || !DPV->isDbgAssign())
+        It.dropOneDbgValue(&DR);
   BB->splice(BI->getIterator(), ThenBB, ThenBB->begin(),
              std::prev(ThenBB->end()));
 
diff --git a/llvm/test/Transforms/SimplifyCFG/branch-fold-dbg.ll b/llvm/test/Transforms/SimplifyCFG/branch-fold-dbg.ll
index 833d72773cdc83..48e831b4206453 100644
--- a/llvm/test/Transforms/SimplifyCFG/branch-fold-dbg.ll
+++ b/llvm/test/Transforms/SimplifyCFG/branch-fold-dbg.ll
@@ -26,10 +26,11 @@ define i1 @foo(i32) nounwind ssp !dbg !0 {
 ; CHECK-NEXT:    [[TMP8:%.*]] = icmp slt i32 [[TMP0]], 0
 ; CHECK-NEXT:    [[SPEC_SELECT:%.*]] = select i1 [[OR_COND2]], i1 false, i1 [[TMP8]], !dbg [[DBG7]]
 ; CHECK-NEXT:    br label [[COMMON_RET]], !dbg [[DBG7]]
+; CHECK-NOT: BB4
 ; CHECK:       common.ret:
 ; CHECK-NEXT:    [[COMMON_RET_OP:%.*]] = phi i1 [ false, [[ENTRY:%.*]] ], [ [[SPEC_SELECT]], [[BB2]] ]
 ; CHECK-NEXT:    ret i1 [[COMMON_RET_OP]], !dbg [[DBG14:![0-9]+]]
-;
+
 Entry:
   %1 = icmp slt i32 %0, 0, !dbg !5
   br i1 %1, label %BB5, label %BB1, !dbg !5
@@ -55,6 +56,10 @@ BB3:                                              ; preds = %BB2
 
 BB4:                                              ; preds = %BB3
   %8 = icmp slt i32 %0, 0, !dbg !5
+  ;; Manually insreted intrinsics; these should get deletd when this block is
+  ;; folded into BB2.
+  call void @llvm.dbg.value(metadata ptr null, metadata !7, metadata !DIExpression()), !dbg !12
+  call void @llvm.dbg.label(metadata !18), !dbg !12
   ret i1 %8, !dbg !14
 
 BB5:                                              ; preds = %BB3, %BB2, %BB1, %Entry
@@ -84,3 +89,4 @@ declare void @llvm.dbg.value(metadata, metadata, metadata) nounwind readnone
 !15 = !DIFile(filename: "a.c", directory: "/private/tmp")
 !16 = !{i32 1, !"Debug Info Version", i32 3}
 !17 = !{i32 2, !"Dwarf Version", i32 4}
+!18 = !DILabel(scope: !0, name: "label", file: !1, line: 1)

[jmorse]

LGTM, although the condition is a bit manky.

[jmorse]

Is this condition going to become !isa<DbgAssign> in the future-class-refactor? It's worth commenting on what you're filtering for seeing how the current arrangement makes it non-obvious.

[OCHyams]

Is this condition going to become !isa in the future-class-refactor?

It may, I'm not sure we ever landed on a decision as to whether we should extract the 3 DPValue kinds into their own classes too. But yes I can add a comment, SGTM