[PATCH] D128192: [GlobalISel][DebugInfo] Propagate debug location for localized constants

[Paul Robinson via Phabricator via llvm-commits]

probinson added a comment.

The case I was having a flashback to was FastISel, which _used to_ put all the constants at the top of the current block, with no debug info.  This was reworked multiple times over the years, and at this point it no longer behaves that way--constants are materialized per IR instruction.

//This// patch (now that I look at it in detail) is doing something else entirely.  The pass was already moving constant-materializing instructions to be adjacent to the user; the change here is, having done that motion, if there's only one use, set the debug info of the moved instruction.  That seems entirely reasonable.

It just happens that, as a special case, sometimes the moved instruction would end up at the top of a block, and IIRC AsmPrinter will deliberately emit line 0 for a first-in-block instruction that has no source location. (This protects the first-in-block instructions from incorrectly inheriting the source location of the physically preceding block.) With this patch, that instruction will have the same source location as its user, which seems like a clear improvement.

I apologize for being triggered on all this stuff, and not recognizing what was going on sooner.  FWIW, LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128192