[PATCH] D70318: Recover debug intrinsics when killing duplicate or empty basic blocks

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a subscriber: efriedma.
vsk added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:1046
+    // undef since its value is unknown.
+    for (auto DI = BB->begin(); isa<DbgInfoIntrinsic>(DI); ++DI)
+      if (auto DVI = cast<DbgVariableIntrinsic>(DI)) {
----------------
I had to squint at this for a while to figure out why it's sufficient. Could you clear this up by writing the loop as `for (auto I : *BB)`, then asserting `isa<DbgInfoIntrinsic>(&I) || I.isTerminator()`?


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:1052
+                   DVI->getContext(),
+                   ValueAsMetadata::get(UndefValue::get(V->getType()))));
+        DVI->moveBefore(Succ->getFirstNonPHI());
----------------
Could you add a new helper to Local.h to share this wrap-in-undef logic with `llvm::replaceDbgUsesWithUndef` (above)?


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:1284
+
+  SkipNonIdenticalDbgInfo(I1, I2);
+
----------------
Is any special/extra work needed to handle duplicate dbg.values for a DebugVariable that's already present in the UndefDVIs map? The first one that's inserted into the map is hoisted and converted to an 'undef' -- what happens to the rest?


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:1401
+                 DVI->getContext(),
+                 ValueAsMetadata::get(UndefValue::get(V->getType()))));
+    }
----------------
You can use the set-to-undef helper here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70318