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

Wed Nov 15 06:53:35 PST 2023

llvmbot wrote:




@llvm/pr-subscribers-debuginfo

Author: Jeremy Morse (jmorse)

<details>
<summary>Changes</summary>

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 scenariso 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).

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


7 Files Affected:

- (modified) llvm/lib/IR/DebugInfo.cpp (+1) 
- (modified) llvm/lib/Transforms/Scalar/ADCE.cpp (+10) 
- (modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (+18) 
- (modified) llvm/test/Transforms/ADCE/adce-salvage-dbg-value.ll (+1) 
- (modified) llvm/test/Transforms/ADCE/debug-info-intrinsic.ll (+1) 
- (modified) llvm/test/Transforms/SimplifyCFG/return-merge.ll (+3) 
- (modified) llvm/test/Transforms/SimplifyCFG/speculate-dbgvalue.ll (+1) 


``````````diff

diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index 390a27c4bc0c4dd..2f35abd5bae0a5d 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -525,6 +525,7 @@ bool llvm::stripDebugInfo(Function &F) {
         // DIAssignID are debug info metadata primitives.
         I.setMetadata(LLVMContext::MD_DIAssignID, nullptr);
       }
+      I.dropDbgValues();
     }
   }
   return Changed;
diff --git a/llvm/lib/Transforms/Scalar/ADCE.cpp b/llvm/lib/Transforms/Scalar/ADCE.cpp
index 24354211341f822..9af275a9f4e204a 100644
--- a/llvm/lib/Transforms/Scalar/ADCE.cpp
+++ b/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;
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 4dae52a8ecffdf6..df2fd700a105e71 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/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()));
 
@@ -5179,6 +5184,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()) {
@@ -5195,6 +5209,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();
diff --git a/llvm/test/Transforms/ADCE/adce-salvage-dbg-value.ll b/llvm/test/Transforms/ADCE/adce-salvage-dbg-value.ll
index ac2fac9832aa616..8af6f01e4f26fc1 100644
--- a/llvm/test/Transforms/ADCE/adce-salvage-dbg-value.ll
+++ b/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"
diff --git a/llvm/test/Transforms/ADCE/debug-info-intrinsic.ll b/llvm/test/Transforms/ADCE/debug-info-intrinsic.ll
index 34c09e2c6f6d36e..5d87a1cbbe0cb44 100644
--- a/llvm/test/Transforms/ADCE/debug-info-intrinsic.ll
+++ b/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
diff --git a/llvm/test/Transforms/SimplifyCFG/return-merge.ll b/llvm/test/Transforms/SimplifyCFG/return-merge.ll
index cd8e88eb6e1e6a3..287a112008d0c9a 100644
--- a/llvm/test/Transforms/SimplifyCFG/return-merge.ll
+++ b/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:
diff --git a/llvm/test/Transforms/SimplifyCFG/speculate-dbgvalue.ll b/llvm/test/Transforms/SimplifyCFG/speculate-dbgvalue.ll
index eb62d6c613c9091..ba26c70962d7303 100644
--- a/llvm/test/Transforms/SimplifyCFG/speculate-dbgvalue.ll
+++ b/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:
 ;

``````````

</details>


https://github.com/llvm/llvm-project/pull/72399
