[PATCH] D71478: [BasicBlockUtils] Add utility to remove redundant dbg.value instrs

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

This is really nice.



================
Comment at: llvm/lib/Transforms/Scalar/DCE.cpp:105
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    AU.setPreservesCFG();
+  }
----------------
Does this imply that all analyses are preserved? If not, should it?


================
Comment at: llvm/lib/Transforms/Utils/BasicBlockUtils.cpp:427
+  // identical mapping earlier in the basic block.
+  MadeChanges |= removeRedundantDbgInstrsUsingForwardScan(BB);
+
----------------
Could you include a short example that demonstrates why neither the forward nor backward scans alone are sufficient to eliminate all redundancies? Maybe:

```
1) dbg.value X1, "x", FragmentX1
<block of instructions, none being "dbg.value ..., "x", ...">
2) dbg.value X1, "x", FragmentX1
3) dbg.value X2, "x", FragmentX2
4) dbg.value X1, "x", FragmentX1
5) dbg.value X1, "x", FragmentX1
```

IIUC the backwards pass would delete (4), and the forwards pass would delete (2). Not sure if that's minimal.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71478/new/

https://reviews.llvm.org/D71478