[PATCH] D52887: [DebugInfo][Dexter] Unreachable line stepped onto after SimplifyCFG.

[Vedant Kumar via Phabricator via llvm-commits]

vsk added inline comments.


================
Comment at: include/llvm/Transforms/Utils/Local.h:454
+/// their debug intrinsic instructions are removed.
+void promotePHINodes(BasicBlock *DomBlock,Instruction *InsertPt,
+                     BasicBlock *IfBlock);
----------------
I think function name and comments here could be clearer.

The action seems closer to 'hoisting' than 'promoting'. And the function is hoisting all of the instructions in the IfBlock, not just the phis. How about: 'hoistAllInstructionsInto'?


================
Comment at: lib/Transforms/Utils/Local.cpp:2535
+  // Since we are moving the instructions out of its basic block, we do not
+  // retain their original debug location and debug intrinsic instructions.
+  // Doing so would degrade the debugging experience and adversely affect the
----------------
aprantl wrote:
> Please also explain why if possible. I think the reason is different for DILoctions and dbg.values.
Right. Moving locations around degrades single-stepping and profiling. Moving dbg.values around makes variable updates appear re-ordered in a debug session.


Repository:
  rL LLVM

https://reviews.llvm.org/D52887