This patch is a follow-up to D58238, where the discussion asked in what circumstances we needed to insert undef DBG_VALUEs into a block when sinking instructions. The answer was (to my mind) "only when it would re-order the appearance of variable assignments". This patch attempts to implement that.
I then realised that I'd misread some of Bjorns question there, so leaving this as a WIP with no reviewers until I'm certain this is a good idea.
The overall idea here is to keep a MachineBasicBlock::iterator that points at (or after) the last DBG_VALUE in a block. We can then identify whether DBG_VALUEs that are to be sunk, sink past the last one, which would mean assignments were re-ordered. In the worst case scenario we'll end up iterating over each basic block twice instead of once, which IMO is a better trade-off than anything that allocates memory.
If a set of DBG_VALUEs sink without any other DBG_VALUE being between them and the end of the block, for example:
blah: %0 = tosink... DBG_VALUE %0, "var1" %1 = inst1... DBG_VALUE %0, "var2" %2 = inst2... DBG_VALUE %0, "var3" br i1 %cond, label %dest, label %nope
and if "tosink" were sunk to another block, we would sink all those DBG_VALUEs into the destination block like this:
blah: %1 = inst1... %2 = inst2... br i1 %cond, ... dest: %0 = tosink... DBG_VALUE %0, "var1" DBG_VALUE %0, "var2" DBG_VALUE %0, "var3" [...] nope: DBG_VALUE undef, "var1" DBG_VALUE undef, "var2" DBG_VALUE undef, "var3" [...]
This reduces the number of sets of of different variable values you can see, because var1/var2/var3 all transition at the same time. This it the correct behaviour IMO because:
- We can't cover the 'blah' block with %0 regardless
- We're keeping var1/var2/var3 alive as long as possible in the 'blah' block
Wheras if there were a DBG_VALUE %2, "var4" after 'inst2', we would get:
blah: %1 = inst1... %2 = inst2... DBG_VALUE undef, "var1" DBG_VALUE undef, "var2" DBG_VALUE %2, "var4" br i1 %cond, ... dest: %0 = tosink... DBG_VALUE %0, "var1" DBG_VALUE %0, "var2" DBG_VALUE %0, "var3" [...] nope: DBG_VALUE undef, "var3" [...]
With var1/var2 leaving undef DBG_VALUEs in place in 'blah', to avoid earlier assignments to var1/var2 reordering with the assignment to var4.
I've split copy-propagation into its own function and applied it before sinking, to avoid awkward interactions with the "last DBG_VALUE in block" iterator. Any DBG_VALUE that can be copy-propagated doesn't need to be sunk anyway, and livedebugvalues should observe the sunk copy and keep the variable alive there.
- postra-ignore-dbg-instrs.mir: the DBG_VALUE gets copy-prop'd now, and the test is only checking that the debug instruction doesn't affect codegen, so I've deleted the debug check line.
- pr38952.mir: this is checking that non-immediately-following DBG_VALUEs get sunk. With this patch, the example gets copy-prop'd. I've changed the DBG_VALUE to refer to a subregister instead (which gets sunk), and test that not-immediately-following DBG_VALUEs get found.
- machinesink.mir: as we're no longer re-ordering assignments, a sunk DBG_VALUE no longer leaves an undef DBG_VALUE behind.
- postra-subreg-sink.mir: Two DBG_VALUEs no longer leave undef's behind, one only gets copy-prop'd, not sunk and copy-prop'd.
- sink-leaves-undef.mir: insert a DBG_VALUE that would lead to ordering reassignment, to test that an undef DBG_VALUE is generated