[PATCH] [RegisterScavenger] Fix handling of predicated instructions

[Quentin Colombet]

> On Jun 8, 2015, at 1:04 PM, Tobias Edler von Koch <tobias at codeaurora.org> wrote:
> 
> 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?

That’s OK.

Thanks,
-Quentin

> 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 <mailto: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.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150608/94e7f0e6/attachment.html>