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

Ramana Radhakrishnan ramana.radhakrishnan@foss.arm.com
Fri Jul 31 10:54:00 GMT 2015


>>
>>
>> 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 ? 

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"
> 



More information about the Gcc-patches mailing list