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
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.
For example, even without this patch it doesn't get used for:
int
foo (int a)
{
return a > 5 ? -a : 8;
}
Bottom line: I'll have a look at codegen differences in SPEC to see if there's
codegen degradation
Thanks,
Kyrill
Not enough coffee in system yet.
regards
Ramana
Kyrill
+ "TARGET_32BIT"
+ "#"
+ "&& reload_completed"
+ [(cond_exec (match_op_dup 4 [(match_dup 3) (const_int 0)])
+ (set (match_dup 0) (neg:SI (match_dup 2))))]
+ ""
[(set_attr "conds" "use")
- (set_attr "length" "4,8,8")
- (set_attr "type" "logic_shift_imm,multiple,multiple")]
+ (set_attr "length" "4")
+ (set_attr "arch" "t2,32")
+ (set_attr "enabled_for_depr_it" "yes,no")
+ (set_attr "type" "logic_shift_imm")]
)
(define_insn "*ifcompare_move_neg"
@@ -10097,21 +10100,34 @@ (define_insn "*ifcompare_move_neg"
(set_attr "type" "multiple")]
)
-(define_insn "*if_move_neg"
- [(set (match_operand:SI 0 "s_register_operand" "=r,r,r")
+(define_insn_and_split "*if_move_neg"
+ [(set (match_operand:SI 0 "s_register_operand" "=l,r")
(if_then_else:SI
(match_operator 4 "arm_comparison_operator"
[(match_operand 3 "cc_register" "") (const_int 0)])
- (match_operand:SI 1 "arm_not_operand" "0,?rI,K")
- (neg:SI (match_operand:SI 2 "s_register_operand" "r,r,r"))))]
- "TARGET_ARM"
- "@
- rsb%D4\\t%0, %2, #0
- mov%d4\\t%0, %1\;rsb%D4\\t%0, %2, #0
- mvn%d4\\t%0, #%B1\;rsb%D4\\t%0, %2, #0"
+ (match_operand:SI 1 "arm_not_operand" "0,0")
+ (neg:SI (match_operand:SI 2 "s_register_operand" "l,r"))))]
+ "TARGET_32BIT"
+ "#"
+ "&& reload_completed"
+ [(cond_exec (match_dup 5)
+ (set (match_dup 0) (neg:SI (match_dup 2))))]
+ {
+ machine_mode mode = GET_MODE (operands[3]);
+ rtx_code rc = GET_CODE (operands[4]);
+
+ if (mode == CCFPmode || mode == CCFPEmode)
+ rc = reverse_condition_maybe_unordered (rc);
+ else
+ rc = reverse_condition (rc);
+
+ operands[5] = gen_rtx_fmt_ee (rc, VOIDmode, operands[3], const0_rtx);
+ }
[(set_attr "conds" "use")
- (set_attr "length" "4,8,8")
- (set_attr "type" "logic_shift_imm,multiple,multiple")]
+ (set_attr "length" "4")
+ (set_attr "arch" "t2,32")
+ (set_attr "enabled_for_depr_it" "yes,no")
+ (set_attr "type" "logic_shift_imm")]
)
Same as above.
(define_insn "*arith_adjacentmem"
