[PATCH] D59272: [DebugInfo] Select debug intrinsic line-numbers more carefully when promoting dbg.declare

[Jeremy Morse via Phabricator via llvm-commits]

jmorse updated this revision to Diff 190626.
jmorse added a comment.

Update to use unknown-locations for everything but promoted store instructions,

rnk wrote:

> @aprantl, I think you are correct for all the code that I can find today. The only thing that matters is the inlinedAt part of the location. But I did see this code that handles dbg.declare, which uses the scope from the location:

So that I understand correctly: the concern was that dbg.values with different DILocations could be seen as different variable instances (bad), but actually only the inlinedAt field is used for that (which is co-incidentally is also a DILocation).

For the avoidance of doubt, the code in getDebugValueLoc always uses the scope and inlinedAt field from the dbg.declare.

> I guess my issue is that using the insertion point just to get "smooth" line numbers seems fragile. If the insertion point isn't a store, I think we'd be better off using an unknown location (line 0). Using the location of the store, assuming it has the right inlinedAt info, as this does, seems good. That just leaves us to ask what should happen at the other dbg.value creation places, call sites, loads, etc.

This update now uses unknown-location for everything except stores (and covers call-sites which I missed before). Using unknown-location for everything would be perfectly fine too IMO, I'm not wed to either idea.


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

https://reviews.llvm.org/D59272

Files:
  lib/Transforms/Utils/Local.cpp
  test/DebugInfo/Generic/dbg-value-lower-linenos.ll
  test/DebugInfo/X86/formal_parameter.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59272.190626.patch
Type: text/x-patch
Size: 10512 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190314/da048046/attachment.bin>