[PATCH] D154353: [DebugInfo][RemoveDIs] Implement behind-the-scenes debug-info maintenance in splice / moveBefore / insertBefore APIs

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando added a comment.

Still LGTM



================
Comment at: llvm/lib/IR/Instruction.cpp:118-119
+
+  // No need to manually update DPValues: if we insert after an instruction
+  // position, then we can never have any DPValues on "this".
+  if (DestParent->IsNewDbgInfoFormat)
----------------
jmorse wrote:
> Orlando wrote:
> > I'm a bit confused about what this comment is saying, please can you explain further (here)?
> My recollection is that there are no scenarios where DPValues can change what instruction they're attached to as a result of insertAfter, for example:
> 
>     call dbg.value
>     %foo = add
>     call dbg.value
>     %bar = sub
>     call dbg.value
> 
> Inserting after %foo or after %bar doesn't cause an instruction be inserted inbetween dbg.values and the next instruction. All the dbg.values continue to "happen" at the next instruction, and that's preserved using DPValues too.
> 
> Of course, if the insert position of an instruction is a dbg.value intrinsic then this is no longer true. Not allowing people to do that is one of the design goals though: doing that is always a bug.
That makes sense, though I still find the code comment confusing. If there's anything you can do to make it clearer that might help. YMMV


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154353