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
