[PATCH] D31188: [AntiDepBreaker] Use liveins as well in StartBlock

[Tim Shen via Phabricator via llvm-commits]

timshen added a comment.

In https://reviews.llvm.org/D31188#718616, @MatzeB wrote:

> >> I also don't understand the fix here: Adding live-in registers when the code here appears to be searching for live-out registers seem like an odd fix to me (I can see how it helps though as it will just mark more registers as occupied).
> > 
> > This is based on my inderstanding on "ideal liveins()" I mentioned in the bug:
> > 
> >   /* Returns all live-ins, including the unspilled callee-saved registers. */
> >   iterator_range<concat_iterator<...>> liveins();
> >    
> > 
> > The fix turns "Pristine.test(Reg)" into "Pristine.test(Reg) || BB.IsLiveIn(Reg)" as a refinement (to match the ideal definition of liveins()).
> > 
> > It means that not only those callee-saved registers (we are iterating over only them) who don't have spill slots are considered, but also those who “have spill slots but not spilled”.
>
> `Pristine.test(Reg) || BB.IsLiveIn(Reg)` would make a lot of sense when searching for **live-ins**. But the comments around it indicate it is actually searching for **live-outs**?


Not for all live-outs, but only for "live-out callee-saved registers".

The code wants to do something on all non-spilled callee-saved registers that are live-outs of the current block. Without considering shrink wrapping, I picture that the correct behavior should be:

1. Epilogue blocks and returning blocks have all callee-saved registers live-out. Handle all of them.
2. Prologue blocks have all callee-saved registers that are spilled. Handle callee-saved registers that are pristine.
3. For the rest of the blocks, a callee-saved register is either not live, or live-through. Handle live-in callee-saved registers.

The behavior was matching my picture, only if all epilogue blocks are returning blocks.

After considering shrink wrapping, things change. Ideally:

1. All blocks before prologue, excluding prologue, have all callee-saved registers live-out.
2. All blocks after epilogue, including epilogue, have all callee-saved registers live-out.
3. For the rest of the blocks, a callee-saved register is either not live, or live-through; pristine registers are precisely live-out callee-saved registers.

Let's see what information we have (I haven't verified yet):

1. For all blocks before prologue, including prologue, shrink wrapping marks all callee-saved registers live-in.
2. For all blocks after epilogue, excluding epilogue, shrink wrapping marks all-callee-saved registers live-in.

I think my fix is wrong about prologues and epilogues, but correct for other blocks. I think that your suggestion is the correct fix, that is looking at the live-ins of the successors. I'll do that.

Do you think that we should keep this patch in the mean time? This patch makes some of the correct cases overly-conservative (but not wrong), and fixes some of the wrong cases. It doesn't make any correct cases wrong. Feel free to revert it if you don't think so.

I'm on vacation until April 15th, so I'll work on it after that. :)


https://reviews.llvm.org/D31188