[PATCH] D66663: [DebugInfo] LiveDebugValues should always revisit backedges if it skips them

[Jeremy Morse via Phabricator via llvm-commits]

jmorse created this revision.
jmorse added reviewers: aprantl, vsk, wolfgangp.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

LiveDebugValues currently ignores loop backedges on the first iteration -- this is because any variable locations that are live-through a loop need to be propagated in regardless. Otherwise, the unvisited backedge would always invalidate any potential live-through variable locations.

However: LiveDebugValues only revisits blocks if the out-locations of a predecessor changes. This creates situations like the "baz" function added in this patch:

- Location of `!103` enters the loop block as %bar, which is taken on faith by ignoring the backedge,
- Location of `!103` changes in the loop block to constant-zero, which should later invalidate the in-location for the loop through the backedge, like it does in the "foo" function,
- However, the out-locations of the loop block are not changed as the result of propagation, therefore the loop block is never re-visited.

The net effect of which is that we took the `!103 == %bar` location on faith, and as the backedge is not seen, it's kept forever. (This doesn't happen in the "foo" function in the same file because I've deliberately set that up to have an out-location change, to continue propagation). Some of this is behind PR38997.

This patch tries to rectify this by, whenever we ignore a backedge, forcing the ignoring block to be revisited at least once. This rectifies the immediate problem. The previous discussion about how this is supposed to work seems to be here [0]: I'm not completely convinced that what we have right now will always get the right answer because, as Vikram puts half way through that thread, getting a full answer requires initialising every block as having all variables in all locations. However, with this patch I think we get more accurate locations, so it's a step in the right direction.

On a clang-3.4 build we drop a single variable location; and the scope-bytes-covered statistic ticks up 0.2%. This is mildly counter-intuitive as I would expect more locations to be invalidated. I'm hoping this is just complex location interactions, with fewer wrong locations it might be easier to prove there are more right ones, currently looking closer.

I've added a third function in the test file that positively checks a variable location that's live-through a loop has its location preserved, just in case we mess with this further.

In the DW_AT_location-reference.ll test that changes, placeDbgValues moves the non-constant variable locations into the middle of the loops. Previously we marked the first instruction of each loop body as having a constant value (which would be wrong), which doesn't occur now -- in fact no instruction gets a constant-value of "x", hence the fewer location list entries. This test says it only cares about the fact that "x" moves around though, so I believe it still works (TM).

[0] https://groups.google.com/d/msg/llvm-dev/nlgxIfDQnhs/c55kcncIAAAJ


Repository:
  rL LLVM

https://reviews.llvm.org/D66663

Files:
  lib/CodeGen/LiveDebugValues.cpp
  test/DebugInfo/X86/DW_AT_location-reference.ll
  test/DebugInfo/X86/live-debug-values-remove-range.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D66663.216874.patch
Type: text/x-patch
Size: 7869 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190823/ffed906a/attachment.bin>