Re: [PATCH][ARM][2/3] Make if_neg_move and if_move_neg into insn_and_split

[Ramana Radhakrishnan <ramana dot radhakrishnan at foss dot arm dot com>]

On 31/07/15 11:49, Kyrill Tkachov wrote:
> 
> On 31/07/15 11:34, Ramana Radhakrishnan wrote:
>>>>
>>>> So, we have a predicate that doesn't cover all the constraints - in this case aren't we forcing everything into operand0. What happens if we just delete this pattern instead of turning it into an insn_and_split - after all we have other parts of the backend where conditional negates and conditional moves will be caught and cond-exec probably post dates some of these if-then-else patterns.
>>> Hmmm yes, I think operand 1 should be tightened to s_register_operand.
>>> The reason I want this pattern is so that I can expand to it in patch 3/3 where I want to create
>>> a conditional negate expression. However, I can't just emit a COND_EXEC at expand time. I found that
>>> reload doesn't handle the dataflow through them properly. With this pattern I can carry the if_then_else
>>> around and split it into the conditional negate only after reload when it's safe.
>> But don't we loose because the immediate alternatives have been lost ? i.e. the original pattern allowed us to express conditional negates where the else condition was a move of an immediate. Thus one didn't require an additional register. Or are you arguing that this is no longer required ?
> 
> I am arguing that this is no longer required. In the original pattern the cases where operand 1 is an
> immediate just outputs:
> 
> mov%D4\\t%0, %1\;rsb%d4\\t%0, %2, #0
> or
> mvn%D4\\t%0, #%B1\;rsb%d4\\t%0, %2, #0

As I said not enough coffee ;) You'll end up getting an unconditional move followed by a conditional neg, so not terrible but may be a bit more work for LRA todo.

> 
> It doesn't do anything smart.
> I can build SPEC2006 with and without this patch to check for suspect code differences, but I suspect
> there won't be much that matches it.
> 

A sanity check is fine - modulo that it's ok to go in.


regards
Ramana