[PATCH] D59027: [WIP][DebugInfo] When sinking DBG_VALUEs, only insert undef DBG_VALUEs if sinking would re-order assignments

[Jeremy Morse via Phabricator via llvm-commits]

jmorse created this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This patch is a follow-up to D58238 <https://reviews.llvm.org/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.

Test changes:

- 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


Repository:
  rL LLVM

https://reviews.llvm.org/D59027

Files:
  lib/CodeGen/MachineSink.cpp
  test/CodeGen/X86/postra-ignore-dbg-instrs.mir
  test/CodeGen/X86/pr38952.mir
  test/DebugInfo/MIR/X86/machinesink.mir
  test/DebugInfo/MIR/X86/postra-subreg-sink.mir
  test/DebugInfo/MIR/X86/sink-leaves-undef.mir

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59027.189476.patch
Type: text/x-patch
Size: 17905 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190306/3f978007/attachment.bin>