[PATCH] D58238: [DebugInfo] MachineSink: Insert undef DBG_VALUEs when sinking instructions, try to forward copies

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.
Herald added a subscriber: ormris.

I produced another patch (D59027 <https://reviews.llvm.org/D59027>, work-in-progress) that only creates undef DBG_VALUEs when the order of assignments would change... however I then realised I'd misunderstood Bjorns question here:

In D58238#1406919 <https://reviews.llvm.org/D58238#1406919>, @bjope wrote:

> It might be that we sink past instructions that has been sunk earlier (or simply been re-scheduled at ISel etc), so it could be that we restore the source order when sinking.  Or we could be sinking past some constant materialisation that has no associated source line. Etc.
>
> After ISel we could have something like this:
>
>   %1 = LOAD ...
>   DBG_VALUE %1, %noreg, "x", ...
>   %2 = AND ...
>   %3 = ADD %1, 3
>   DBG_VALUE %3, %noreg, "x", ...
>   %5 = SUB 0, 0
>
>
> or we could just as well have the SUB before the ADD afaict
>
>   %1 = LOAD ...
>   DBG_VALUE %1, %noreg, "x", ...
>   %2 = AND ...
>   %5 = SUB 0, 0
>   %3 = ADD %1, 3
>   DBG_VALUE %3, %noreg, "x", ...
>
>
> Sinking the ADD past the SUB would introduce an undef location for "x", when done by MachineSink. But not if the instructions were emitted in that order already at ISel.


If I understand you correctly, this actually isn't a case that MachineSink deals with: MachineSink sinks insts from parent to child basic blocks, to remove partially redundant computations. Take this example [0], where the load of %ab is only used down one path, MachineSink moves it to only execute on the path which actually uses the load. With that in mind it might be clearer why undefs become necessary: if the DBG_VALUE moves to a different block, one of the source-level assignments to the corresponding variable has disappeared from other paths through the program. Those paths should have the variable appear ``optimised out'' until the variable gets a new location, to stop the old location being used on the other paths which would present a stale variable value (effectively this re-orders the appearance of assignments).

Using the example quoted above, if ADD were sunk into a _successor_ block, then an undef DBG_VALUE would have to go somewhere, to terminate the earlier location of "x" in the other successor blocks. (NB, MachineSink doesn't operate if there's only one successor).

[0] https://github.com/llvm-mirror/llvm/blob/master/test/CodeGen/X86/MachineSink-DbgValue.ll


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

https://reviews.llvm.org/D58238