[PATCH] D56788: [DebugInfo][InstCombine] Prefer salvaging dbg.values over sinking them

[Jeremy Morse via Phabricator via llvm-commits]

jmorse marked 4 inline comments as done.
jmorse added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:3110
     if (DII->getParent() == SrcBlock) {
-      DII->moveBefore(&*InsertPos);
-      LLVM_DEBUG(dbgs() << "SINK: " << *DII << '\n');
+      // dbg.value is local to sunk inst, see if we can salvage it. Clone a new
+      // copy of the instruction: on success we need both salvaged and
----------------
aprantl wrote:
> "is local to" -> is in the same basic block ?
Folding into commit,


================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:3123
+        DII->setOperand(0, MetadataAsValue::get(DII->getContext(),
+                                                ValueAsMetadata::get(Undef)));
+      } else {
----------------
aprantl wrote:
> Should we add a `replaceWithUndef` method to DebugInfoIntrinsicInst?
We definitely should -- this should be a common operation by optimisations. (I'll file a follow-up at some point).


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

https://reviews.llvm.org/D56788