On 31/07/15 12:04, Ramana Radhakrishnan wrote:
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.
As mentioned earlier I'll change operand 1 to be use the s_register_operand
predicate and re-test.
Is that change ok as well?
Thanks,
Kyrill
regards
Ramana
