[PATCH] [RegisterScavenger] Fix handling of predicated instructions

[Tobias Edler von Koch]

Thanks Matthias!

The bug is triggered by code that hasn't been upstreamed yet, so I
can't provide a test case right now. I'm happy to add one once that
pass becomes public. Would that be OK? I can't think of an easy way to
produce ARM or Hexagon code that triggers this in the current state.

Tobias

On Fri, 5 Jun 2015 18:50:59 -0700 Quentin Colombet
<qcolombet at apple.com> wrote:

> Ah great!
> 
> Thanks Matthias!
> 
> Tobias, please add a testcase to your patch and you can push your changes.
> 
> Cheers,
> Q.
> > On Jun 5, 2015, at 5:08 PM, Matthias Braun <mbraun at apple.com> wrote:
> > 
> > I changed IfConversion in r192482 (and following fixes) to produce kill flags that are correct independently of predicates. I think updating the register scavenger for this is fine.
> > 
> > - Matthias
> > 
> >> On Jun 5, 2015, at 4:10 PM, Tobias Edler von Koch <tobias at codeaurora.org> wrote:
> >> 
> >> On Fri, 5 Jun 2015 15:55:16 -0700 Quentin Colombet
> >> <qcolombet at apple.com> wrote:
> >> 
> >>> 
> >>> Hold on, isn’t the change you are proposing supposed to allow that, i.e., reuse a register after it has a kill flag set?
> >>> 
> >>> I.e., this patch creates the very case I was saying is broken and that the verifier won’t complain about.
> >>> 
> >>> Consider:
> >>> 
> >>> BB1
> >>> a =
> >>> <predicate> = a<kill>
> >>> <predicate> return
> >>> 
> >>> BB2 <livein a>
> >>> = a
> >>> 
> >>> With your change, the scavenger would be allowed to do:
> >>> BB1
> >>> a =
> >>> <predicate> = a<kill>
> >>> <predicate> return
> >>> // Reuse a
> >>> BB2 <livein a>
> >>> = a <— a is wrong now.
> >>> 
> >> 
> >> Right, sorry, I see what you're saying now. Thanks for clarifying!
> >> 
> >> The problem is that currently it will do:
> >> 
> >> BB1
> >> a =
> >> <predicate> = a<kill>
> >> <predicate> return
> >> // Spill a  <-- not legal per the verifier, because a is killed
> >> // Reuse a
> >> // Reload a
> >> 
> >> BB2 <livein a>
> >> = a 
> >> 
> >> So then there are two solutions to fix the bug
> >> a) Figure out whether if conversion really doesn't set kill flags
> >>  correctly - any other passes that might be affected? If the flags
> >>  really are wrong, this should be fixed.
> >> b) Never scavenge a register that's killed by a predicated
> >>  instruction... I guess this would be OK as work-around.
> >> 
> >> Tobias
> >> 
> >> -- 
> >> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
> >> Linux Foundation Collaborative Project.
> >> 
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > 
> 



-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project.