[PATCH] [RegisterScavenger] Fix handling of predicated instructions

[Quentin Colombet]

Hi Tobias,

> On Jun 5, 2015, at 3:41 PM, Tobias Edler von Koch <tobias at codeaurora.org> wrote:
> 
> Hi Quentin,
> 
> Sorry for not replying earlier, this slipped through the net somehow!
> 
> On Tue, 28 Apr 2015 09:49:18 -0700 Quentin Colombet
> <qcolombet at apple.com> wrote:
> 
>>> 
>>> I don't know of a specific commit. But here's my reasoning:
>>> 
>>> If kill flags were incorrectly set on predicate instructions, wouldn't
>>> we see uses of those registers after they were killed? The
>>> MachineVerifier would flag these up.
>> 
>> Not necessarily. IIRC, the MachineVerifier relies on the live-in information which is updated independently than the kill flags. The consequence is as long as the libe-in information is correct and the uses in different block, the kill flag may be wrong and the MachineVerifier will not complain.
>> 
>> Could you double check how the MachineVerifier actually work with the kill flags?
> 
> Correct, the MachineVerifier uses live-in information to determine
> which registers are live on basic block entry. But so does the
> RegisterScavenger. 

So if those are not in sync, we may create wrong code. And my point is that I am not sure if-conversion updates this information correctly. 

> 
> This bug is about what happens locally *within* a basic block. And
> that's where I can see no situation where you could legally have
> a register that has a <kill> flag set and is then used (still within
> the same BasicBlock).

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.

Cheers,
-Quentin



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