[PATCH] D157158: Do not merge debug locations when sinking instructions

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

Orlando added a comment.

The result of this change is that debug locations are preserved after sinking. I think this is a situation where the docs <https://llvm.org/docs/HowToUpdateDebugInfo.html#when-to-preserve-an-instruction-location> specify that we should not preserve the source location of an instruction. Here is an excerpt from the linked doc explaining when we should preserve debug info:

> A transformation should preserve the debug location of an instruction if the instruction either remains in its basic block, or if its basic block is folded into a predecessor that branches unconditionally. The APIs to use are IRBuilder, or Instruction::setDebugLoc.

And an example of when we shouldn't:

> Examples of transformations for which this rule does not apply include:
> LICM. E.g., if an instruction is moved from the loop body to the preheader, the rule for dropping locations applies.

To be honest I'm not sure if we should be merging locations at all (current behaviour), but it might be fine due to the following additional rule if the definition of "identical" is stretched (merging will preserve the identical parts of the source location).

> In addition to the rule above, a transformation should also preserve the debug location of an instruction that is moved between basic blocks, if the destination block already contains an instruction with an identical debug location.

In short I think the current behaviour lines up with documented expectations. What are your thoughts, and do you have a reproducer where this behaviour causes problems?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157158