[DebugInfo][RemoveDIs] Make dropping variable locations explicit (#72399)

In present-day debug-info, when you delete all instructions, you delete
all their debug-info with it because debug-info is stored in
instructions. With debug-info stored in DPValue objects however,
deleting instructions causes DPValue objects to clump together into a
large blob of debug-info that hangs around in the block, as nothing has
explicitly deleted it.

To restore this behaviour, scatter calls to dropDbgValues around in
places that used to delete chunks of dbg.values, for example during
stripDebugInfo and in the code that deletes everything after an
Unreachable instruction. DCE is another example.

The tests with --try... added to them are new scenarios where we can now
correctly replicate the "normal" debug-info behaviour. Alas, there's no
explicit test for the opt -strip-debug option though (in dbg.value mode
or DPValue mode).

Author: jmorse

llvm/lib/IR/DebugInfo.cpp

@@ -547,6 +547,7 @@ bool llvm::stripDebugInfo(Function &F) {
         // DIAssignID are debug info metadata primitives.
         I.setMetadata(LLVMContext::MD_DIAssignID, nullptr);
       }
+      I.dropDbgValues();
     }
   }
   return Changed;

llvm/lib/Transforms/Scalar/ADCE.cpp

@@ -544,6 +544,16 @@ ADCEChanged AggressiveDeadCodeElimination::removeDeadInstructions() {
   // value of the function, and may therefore be deleted safely.
   // NOTE: We reuse the Worklist vector here for memory efficiency.
   for (Instruction &I : llvm::reverse(instructions(F))) {
+    // With "RemoveDIs" debug-info stored in DPValue objects, debug-info
+    // attached to this instruction, and drop any for scopes that aren't alive,
+    // like the rest of this loop does. Extending support to assignment tracking
+    // is future work.
+    for (DPValue &DPV : make_early_inc_range(I.getDbgValueRange())) {
+      if (AliveScopes.count(DPV.getDebugLoc()->getScope()))
+        continue;
+      I.dropOneDbgValue(&DPV);
+    }
+
     // Check if the instruction is alive.
     if (isLive(&I))
       continue;

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

@@ -3122,6 +3122,11 @@ bool SimplifyCFGOpt::SpeculativelyExecuteBB(BranchInst *BI,
   }
 
   // Hoist the instructions.
+  // In "RemoveDIs" non-instr debug-info mode, drop DPValues attached to these
+  // instructions, in the same way that dbg.value intrinsics are dropped at the
+  // end of this block.
+  for (auto &It : make_range(ThenBB->begin(), ThenBB->end()))
+    It.dropDbgValues();
   BB->splice(BI->getIterator(), ThenBB, ThenBB->begin(),
              std::prev(ThenBB->end()));
 
@@ -5194,6 +5199,15 @@ bool SimplifyCFGOpt::simplifyUnreachable(UnreachableInst *UI) {
 
   bool Changed = false;
 
+  // Ensure that any debug-info records that used to occur after the Unreachable
+  // are moved to in front of it -- otherwise they'll "dangle" at the end of
+  // the block.
+  BB->flushTerminatorDbgValues();
+
+  // Debug-info records on the unreachable inst itself should be deleted, as
+  // below we delete everything past the final executable instruction.
+  UI->dropDbgValues();
+
   // If there are any instructions immediately before the unreachable that can
   // be removed, do so.
   while (UI->getIterator() != BB->begin()) {
@@ -5210,6 +5224,10 @@ bool SimplifyCFGOpt::simplifyUnreachable(UnreachableInst *UI) {
     // block will be the unwind edges of Invoke/CatchSwitch/CleanupReturn,
     // and we can therefore guarantee this block will be erased.
 
+    // If we're deleting this, we're deleting any subsequent dbg.values, so
+    // delete DPValue records of variable information.
+    BBI->dropDbgValues();
+
     // Delete this instruction (any uses are guaranteed to be dead)
     BBI->replaceAllUsesWith(PoisonValue::get(BBI->getType()));
     BBI->eraseFromParent();

llvm/test/Transforms/ADCE/adce-salvage-dbg-value.ll

@@ -1,5 +1,6 @@
 ;; Check that adce salvages debug info properly.
 ; RUN: opt -passes=adce -S < %s | FileCheck %s
+; RUN: opt -passes=adce -S < %s --try-experimental-debuginfo-iterators| FileCheck %s
 
 ; ModuleID = 'test.ll'
 source_filename = "test.ll"

llvm/test/Transforms/ADCE/debug-info-intrinsic.ll

@@ -1,4 +1,5 @@
 ; RUN: opt -passes=adce -S < %s | FileCheck %s
+; RUN: opt -passes=adce -S < %s --try-experimental-debuginfo-iterators | FileCheck %s
 ; Test that debug info intrinsics in dead scopes get eliminated by -adce.
 
 ; Generated with 'clang -g -S -emit-llvm | opt -passes=mem2reg -inline' at r262899

llvm/test/Transforms/SimplifyCFG/return-merge.ll

@@ -2,6 +2,9 @@
 ; RUN: opt < %s -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S | FileCheck --check-prefixes=CHECK %s
 ; RUN: opt < %s -passes=debugify,simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S | FileCheck --check-prefixes=DBGINFO %s
 
+; RUN: opt < %s -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S --try-experimental-debuginfo-iterators | FileCheck --check-prefixes=CHECK %s
+; RUN: opt < %s -passes=debugify,simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S --try-experimental-debuginfo-iterators | FileCheck --check-prefixes=DBGINFO %s
+
 define i32 @test1(i1 %C) {
 ; CHECK-LABEL: @test1(
 ; CHECK-NEXT:  entry:

llvm/test/Transforms/SimplifyCFG/speculate-dbgvalue.ll

@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -S -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 | FileCheck %s
+; RUN: opt < %s -S -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 --try-experimental-debuginfo-iterators | FileCheck %s
 
 ; This test case was generated from speculate-dbgvalue.c:
 ;