[PATCH] D80052: [docs] Sketch outline for HowToUpdateDebugInfo.rst

[Vedant Kumar via Phabricator via llvm-commits]

vsk marked an inline comment as not done.
vsk added a subscriber: dexonsmith.
vsk added inline comments.


================
Comment at: llvm/docs/HowToUpdateDebugInfo.rst:27
+
+When an ``Instruction`` is deleted, its debug uses change to ``undef``.  This
+is a loss of debug info: the value of a one or more source variables becomes
----------------
TWeaver wrote:
> Hi and thanks for this!
> 
> I'm not sure this line is actually correct.
> 
> It's my assumption that when a value pointed at by an intrinsic is deleted, the intrinsic is left with a valueless ValueAsMetaData() operand.
> 
> This empty ValueAsMetaData points at an empty metadata node, thus leaving us with an 'empty' intrinsic.
> 
> These empty intrinsics are usually removed from the program by later passes that clean up or remove 'junk' from the instruction stream.
> 
> My reading of this line gives me the impression that if I delete a value pointed to by an intrinsic, the intrinsic is auto-magically fixed up to be undef.
> 
> This may not have been your intent but it's how it reads to me.
> 
> I've fixed a bug in the Reassociate pass related to this issue in:
> 
> https://reviews.llvm.org/rG41c3f76dcd0daeec60eddfcb56008a31ad6e8738
Thanks for pointing this out. This seems bad.

A valueless dbg.value is treated as trivially dead (see `wouldInstructionBeTriviallyDead`). So unless specific care is taken to call `salvageDebugInfoOrMarkUndef`, deleting an Instruction -- and then running any instruction simplification helper -- results in inaccurate debug info.

@aprantl @dexonsmith can we change Instruction deletion s.t. any ValueAsMetadata nodes pointing to the dying value change to undef?


================
Comment at: llvm/docs/HowToUpdateDebugInfo.rst:232
+   be precise enough), moving the test to its own file is preferred.
+
+MIR-level transformations
----------------
aprantl wrote:
> Do we want to add a section on SelectionDAG & GlobalISel, DAGCombine, etc?
The info about MIR should generally carry over to GIsel. This probably does need a separate section for SelectionDAG, but I'm not sure how to structure that, so I'll leave it as TODO as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80052