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

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added a subscriber: dberlin.
aprantl added a comment.

> 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.

Conceptually, the analysis uses a lattice with three elements to describe each VarLoc at each point: ⊥ (uninitialized), true (VarLoc is valid), false (VarLoc is not valid). As @dberlin pointed out in the linked thread, we LiveDebugValues *implicitly* represents the (uninitialized) state by ensuring a specific iteration order. (Not joining the unvisited backedge with the incoming edge, is equivalent to joining a set full of ⊥'s with the incoming edge).

> 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,

Makes sense.

> 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,

When the set out OutLocs changes (and in this case it changes from [crumpets=⊥] to [crumpets=0]) all successor blocks (in this case also the block itself because of the back edge) should be added to worklist automatically. Why isn't this happening?

> However, the out-locations of the loop block are not changed as the result of propagation, therefore the loop block is never re-visited.

That I don't understand yet. They change from "no info" -> "some info". What am I missing?

> 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.




Repository:
  rL LLVM

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

https://reviews.llvm.org/D66663