[llvm] [DebugInfo][RemoveDIs] Implement redundant elimination for DPValues (PR #72284)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-debuginfo

Author: Jeremy Morse (jmorse)

<details>
<summary>Changes</summary>

This pass steps through a block forwards and backwards, identifying those variable assignment records that are redundant, and erases them, saving us a decent wedge of compile-time. This patch re-implements it to use the replacement for DbgValueInsts, DPValues, in an almost identical way.

Alas the test I've added the try-remove-dis flag to is the only one I've been able to find that manually runs this pass.

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


2 Files Affected:

- (modified) llvm/lib/Transforms/Utils/BasicBlockUtils.cpp (+63) 
- (modified) llvm/test/Transforms/DCE/dbg-value-removal.ll (+1) 


``````````diff
diff --git a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
index 05ff4efb7b94471..b64029f2560db5f 100644
--- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -382,7 +382,38 @@ bool llvm::MergeBlockSuccessorsIntoGivenBlocks(
 /// - 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 DPValuesRemoveRedundantDbgInstrsUsingBackwardScan(BasicBlock *BB) {
+  SmallVector<DPValue *, 8> ToBeRemoved;
+  SmallDenseSet<DebugVariable> VariableSet;
+  for (auto &I : reverse(*BB)) {
+    for (DPValue &DPV : reverse(I.getDbgValueRange())) {
+      DebugVariable Key(DPV.getVariable(),
+                        DPV.getExpression(),
+                        DPV.getDebugLoc()->getInlinedAt());
+      auto R = VariableSet.insert(Key);
+      // If the same variable fragment is described more than once it is enough
+      // to keep the last one (i.e. the first found since we for reverse
+      // iteration).
+      if (!R.second)
+        ToBeRemoved.push_back(&DPV);
+      continue;
+    }
+    // Sequence with consecutive dbg.value instrs ended. Clear the map to
+    // restart identifying redundant instructions if case we find another
+    // dbg.value sequence.
+    VariableSet.clear();
+  }
+
+  for (auto &DPV : ToBeRemoved)
+    DPV->eraseFromParent();
+
+  return !ToBeRemoved.empty();
+}
+
 static bool removeRedundantDbgInstrsUsingBackwardScan(BasicBlock *BB) {
+  if (BB->IsNewDbgInfoFormat)
+    return DPValuesRemoveRedundantDbgInstrsUsingBackwardScan(BB);
+
   SmallVector<DbgValueInst *, 8> ToBeRemoved;
   SmallDenseSet<DebugVariable> VariableSet;
   for (auto &I : reverse(*BB)) {
@@ -440,7 +471,39 @@ static bool removeRedundantDbgInstrsUsingBackwardScan(BasicBlock *BB) {
 ///
 /// Possible improvements:
 /// - Keep track of non-overlapping fragments.
+static bool DPValuesRemoveRedundantDbgInstrsUsingForwardScan(BasicBlock *BB) {
+  SmallVector<DPValue *, 8> ToBeRemoved;
+  DenseMap<DebugVariable, std::pair<SmallVector<Value *, 4>, DIExpression *>>
+      VariableMap;
+  for (auto &I : *BB) {
+    for (DPValue &DPV : I.getDbgValueRange()) {
+      DebugVariable Key(DPV.getVariable(),
+                        std::nullopt,
+                        DPV.getDebugLoc()->getInlinedAt());
+      auto VMI = VariableMap.find(Key);
+      // Update the map if we found a new value/expression describing the
+      // variable, or if the variable wasn't mapped already.
+      SmallVector<Value *, 4> Values(DPV.location_ops());
+      if (VMI == VariableMap.end() || VMI->second.first != Values ||
+          VMI->second.second != DPV.getExpression()) {
+        VariableMap[Key] = {Values, DPV.getExpression()};
+        continue;
+      }
+      // Found an identical mapping. Remember the instruction for later removal.
+      ToBeRemoved.push_back(&DPV);
+    }
+  }
+
+  for (auto *DPV : ToBeRemoved)
+    DPV->eraseFromParent();
+
+  return !ToBeRemoved.empty();
+}
+
 static bool removeRedundantDbgInstrsUsingForwardScan(BasicBlock *BB) {
+  if (BB->IsNewDbgInfoFormat)
+    return DPValuesRemoveRedundantDbgInstrsUsingForwardScan(BB);
+
   SmallVector<DbgValueInst *, 8> ToBeRemoved;
   DenseMap<DebugVariable, std::pair<SmallVector<Value *, 4>, DIExpression *>>
       VariableMap;
diff --git a/llvm/test/Transforms/DCE/dbg-value-removal.ll b/llvm/test/Transforms/DCE/dbg-value-removal.ll
index 3506546fca28e5f..f8f01120d0a0692 100644
--- a/llvm/test/Transforms/DCE/dbg-value-removal.ll
+++ b/llvm/test/Transforms/DCE/dbg-value-removal.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -S -passes=redundant-dbg-inst-elim | FileCheck %s
+; RUN: opt < %s -S -passes=redundant-dbg-inst-elim --try-experimental-debuginfo-iterators | FileCheck %s
 
 ; All dbg.value with location "!dbg !19" are redundant in the input.
 ; FIXME: We do not handle non-overlapping/overlapping fragments perfectly yet.

``````````

</details>


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