diff mbox

[ARM] Alternatives and type attributes fixes.

Message ID CAD57uCdxCK-D8eHgEQ4KLbipC1h0Rhb==yFd__C4MANa5X8n5A@mail.gmail.com
State New
Headers show

Commit Message

Yvan Roux April 28, 2015, 8:32 a.m. UTC 
Hi,

On 27 April 2015 at 15:58, Yvan Roux <yvan.roux@linaro.org> wrote:
> On 27 April 2015 at 15:20, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
>> Hi Yvan,
>>
>>
>> On 27/04/15 13:25, Yvan Roux wrote:
>>>
>>> Hi,
>>>
>>> This is a follow-up of PR64208 where an LRA loop was due to redundancy
>>> in insn's alternatives.  I've checked all the insns in the arm backend
>>> to avoid latent problems and this patch fixes the issues I've spotted.
>>>
>>> Only thumb2_movsicc_insn contained duplicated alternatives, and the
>>> rest of the changes are related to the type attribute, which were not
>>> accurate or used accordingly to their definition.  Notice that the
>>> modifications have only a limited impact as in most of the pipeline
>>> descriptions, multiple/mov_reg/alu_reg/.. types are used the same way,
>>> only cortex a8 seems to have a real difference between alu or multiple
>>> instruction and mov.
>>>
>>> Bootstrapped and regtested on arm-linux-gnueabihf.  Ok for trunk ?
>>>
>>> Thanks,
>>> Yvan
>>>
>>> 2015-04-27  Yvan Roux<yvan.roux@linaro.org>
>>>
>>>      * config/arm/arm.mb (*arm_movt): Fix type attribute.
>>>      (*movsi_compare0): Likewise.
>>>      (*cmpsi_shiftsi): Likewise.
>>>      (*cmpsi_shiftsi_swp): Likewise.
>>>      (*movsicc_insn): Likewise.
>>>      (*cond_move): Likewise.
>>>      (*if_plus_move): Likewise.
>>>      (*if_move_plus): Likewise.
>>>      (*if_arith_move): Likewise.
>>>      (*if_move_arith): Likewise.
>>>      (*if_shift_move): Likewise.
>>>      (*if_move_shift): Likewise.
>>>      * config/arm/thumb2.md (*thumb2_movsi_insn): Fix type attribute.
>>>      (*thumb2_movsicc_insn): Fix alternative redundancy.
>>>      (*thumb2_addsi_short): Split register and immediate alternatives.
>>>      (thumb2_addsi3_compare0): Likewise.
>>>
>>> insn.diff
>>>
>>>
>>> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
>>> index 164ac13..ee23deb 100644
>>> --- a/gcc/config/arm/arm.md
>>> +++ b/gcc/config/arm/arm.md
>>> @@ -5631,7 +5631,7 @@
>>>     [(set_attr "predicable" "yes")
>>>      (set_attr "predicable_short_it" "no")
>>>      (set_attr "length" "4")
>>> -   (set_attr "type" "mov_imm")]
>>> +   (set_attr "type" "alu_sreg")]
>>>   )
>>
>>
>> For some context, this is the *arm_movt pattern that generates
>> a movt instruction. The documentation in types.md says:
>> "This excludes MOV and MVN but includes MOVT." So using alu_sreg
>> is correct according to that logic.
>> However, don't you want to also update the arm_movtas_ze pattern
>> that generates movt but erroneously has the type 'mov_imm'?
>
> Ha, yes I missed this one ! I'll will update it.
>
>>>     (define_insn "*arm_movsi_insn"
>>> @@ -5919,7 +5919,7 @@
>>>      cmp%?\\t%0, #0
>>>      sub%.\\t%0, %1, #0"
>>>     [(set_attr "conds" "set")
>>> -   (set_attr "type" "alus_imm,alus_imm")]
>>> +   (set_attr "type" "alus_sreg,alus_sreg")]
>>>   )
>>
>>
>> Why change that? They are instructions with immediate operands
>> so alus_imm should be gorrect?
>
> Oups, I certainly misread #0 and %0 this one is correct.
>
>>> @@ -486,12 +486,12 @@
>>>   )
>>>     (define_insn_and_split "*thumb2_movsicc_insn"
>>> -  [(set (match_operand:SI 0 "s_register_operand"
>>> "=l,l,r,r,r,r,r,r,r,r,r")
>>> +  [(set (match_operand:SI 0 "s_register_operand"
>>> "=l,l,r,r,r,r,r,r,r,r,r,r")
>>>         (if_then_else:SI
>>>          (match_operator 3 "arm_comparison_operator"
>>>           [(match_operand 4 "cc_register" "") (const_int 0)])
>>> -        (match_operand:SI 1 "arm_not_operand" "0 ,lPy,0 ,0,rI,K,rI,rI,K
>>> ,K,r")
>>> -        (match_operand:SI 2 "arm_not_operand" "lPy,0 ,rI,K,0 ,0,rI,K
>>> ,rI,K,r")))]
>>> +        (match_operand:SI 1 "arm_not_operand" "0 ,lPy,0 ,0,rI,K,I ,r,rI,K
>>> ,K,r")
>>> +        (match_operand:SI 2 "arm_not_operand" "lPy,0 ,rI,K,0 ,0,rI,I,K
>>> ,rI,K,r")))]
>>>     "TARGET_THUMB2"
>>>     "@
>>>      it\\t%D3\;mov%D3\\t%0, %2
>>> @@ -504,12 +504,14 @@
>>>      #
>>>      #
>>>      #
>>> +   #
>>>      #"
>>>      ; alt 6: ite\\t%d3\;mov%d3\\t%0, %1\;mov%D3\\t%0, %2
>>> -   ; alt 7: ite\\t%d3\;mov%d3\\t%0, %1\;mvn%D3\\t%0, #%B2
>>> -   ; alt 8: ite\\t%d3\;mvn%d3\\t%0, #%B1\;mov%D3\\t%0, %2
>>> -   ; alt 9: ite\\t%d3\;mvn%d3\\t%0, #%B1\;mvn%D3\\t%0, #%B2
>>> -   ; alt 10: ite\\t%d3\;mov%d3\\t%0, %1\;mov%D3\\t%0, %2
>>> +   ; alt 7: ite\\t%d3\;mov%d3\\t%0, %1\;mov%D3\\t%0, %2
>>> +   ; alt 8: ite\\t%d3\;mov%d3\\t%0, %1\;mvn%D3\\t%0, #%B2
>>> +   ; alt 9: ite\\t%d3\;mvn%d3\\t%0, #%B1\;mov%D3\\t%0, %2
>>> +   ; alt 10: ite\\t%d3\;mvn%d3\\t%0, #%B1\;mvn%D3\\t%0, #%B2
>>> +   ; alt 11: ite\\t%d3\;mov%d3\\t%0, %1\;mov%D3\\t%0, %2
>>>     "&& reload_completed"
>>>     [(const_int 0)]
>>>     {
>>> @@ -540,8 +542,8 @@
>>>                                                  operands[2])));
>>>       DONE;
>>>     }
>>> -  [(set_attr "length" "4,4,6,6,6,6,10,10,10,10,6")
>>> -   (set_attr "enabled_for_depr_it" "yes,yes,no,no,no,no,no,no,no,no,yes")
>>> +  [(set_attr "length" "4,4,6,6,6,6,10,8,10,10,10,6")
>>> +   (set_attr "enabled_for_depr_it"
>>> "yes,yes,no,no,no,no,no,no,no,no,no,yes")
>>>      (set_attr "conds" "use")
>>>      (set_attr "type" "multiple")]
>>>   )
>>
>>
>> So, I think here for the first 6 alternatives we can give it a more specific
>> type,
>> like mov_immm or mov_reg, since you're cleaning this stuff up. The
>> instructions in
>> those alternatives are just a mov instruction enclosed in an IT block, so
>> they always
>> have to stick together anyway.
>
> OK I'll change that.
>
>>> @@ -1161,9 +1163,9 @@
>>>   )
>>>     (define_insn "*thumb2_addsi_short"
>>> -  [(set (match_operand:SI 0 "low_register_operand" "=l,l")
>>> -       (plus:SI (match_operand:SI 1 "low_register_operand" "l,0")
>>> -                (match_operand:SI 2 "low_reg_or_int_operand" "lPt,Ps")))
>>> +  [(set (match_operand:SI 0 "low_register_operand" "=l,l,l")
>>> +       (plus:SI (match_operand:SI 1 "low_register_operand" "l,l,0")
>>> +                (match_operand:SI 2 "low_reg_or_int_operand" "l,Pt,Ps")))
>>>      (clobber (reg:CC CC_REGNUM))]
>>>     "TARGET_THUMB2 && reload_completed"
>>>     "*
>>> @@ -1182,7 +1184,7 @@
>>>     "
>>>     [(set_attr "predicable" "yes")
>>>      (set_attr "length" "2")
>>> -   (set_attr "type" "alu_sreg")]
>>> +   (set_attr "type" "alu_sreg,alu_imm,alu_imm")]
>>>   )
>>>     (define_insn "*thumb2_subsi_short"
>>> @@ -1226,10 +1228,10 @@
>>>   (define_insn "thumb2_addsi3_compare0"
>>>     [(set (reg:CC_NOOV CC_REGNUM)
>>>         (compare:CC_NOOV
>>> -         (plus:SI (match_operand:SI 1 "s_register_operand" "l,  0, r")
>>> -                  (match_operand:SI 2 "arm_add_operand"    "lPt,Ps,rIL"))
>>> +         (plus:SI (match_operand:SI 1 "s_register_operand" "l,l,  0,r,r")
>>> +                  (match_operand:SI 2 "arm_add_operand"
>>> "l,Pt,Ps,r,IL"))
>>>           (const_int 0)))
>>> -   (set (match_operand:SI 0 "s_register_operand" "=l,l,r")
>>> +   (set (match_operand:SI 0 "s_register_operand" "=l,l,l,r,r")
>>>         (plus:SI (match_dup 1) (match_dup 2)))]
>>>     "TARGET_THUMB2"
>>>     "*
>>> @@ -1246,8 +1248,8 @@
>>>         return \"adds\\t%0, %1, %2\";
>>>     "
>>>     [(set_attr "conds" "set")
>>> -   (set_attr "length" "2,2,4")
>>> -   (set_attr "type" "alu_sreg")]
>>> +   (set_attr "length" "2,2,2,4,4")
>>> +   (set_attr "type" "alus_sreg,alus_imm,alus_imm,alus_sreg,alus_imm")]
>>>   )
>>>
>>
>>
>> In the other patterns in arm.md you didn't split up the alternatives
>> but instead used an if_then_else in the set_attr_alternative to
>> differentiate
>> the case where the operand is constant.
>> Any particular reason why you split up the alternatives here?
>
> I think that I tried to be consistent with the implementation of
> *thumb2_addsi3_compare0_scratch insn, it is also due to this work was
> interrupted several time and I wasn't really consistent with myself ;)
>
>> In my opinion, having fewer alternatives at the expense of a more complex
>> definition
>> of 'type' is preferable, but someone might have a stronger opinion in the
>> other
>> direction.
>
> I also prefer fewer alternatives, I'll rework the patch that way and
> refactor *thumb2_addsi3_compare0_scratch too (or will do the opposite
> if a good argument comes in that thread).
>
>> The patch looks ok to me otherwise, but please respin with the comments
>> above addressed.

Here is the new patch with the needed modifications.  Rebuilt and
regtested on arm-linux-gnueabihf.

Cheers,
Yvan

2015-04-28  Yvan Roux  <yvan.roux@linaro.org>

    * config/arm/arm.mb (*arm_movt): Fix type attribute.
    (*cmpsi_shiftsi): Likewise.
    (*cmpsi_shiftsi_swp): Likewise.
    (*movsicc_insn): Likewise.
    (*cond_move): Likewise.
    (*if_plus_move): Likewise.
    (*if_move_plus): Likewise.
    (*if_arith_move): Likewise.
    (*if_move_arith): Likewise.
    (*if_shift_move): Likewise.
    (*if_move_shift): Likewise.
    (*arm_movtas_ze): Likewise.
    * config/arm/thumb2.md (*thumb2_movsicc_insn): Fix alternative
redundancy and
    type attributes.
    (*thumb2_movsi_insn): Fix type attribute.
    (*thumb2_addsi_short): Likewise.
    (thumb2_addsi3_compare0): Likewise.
    (*thumb2_addsi3_compare0_scratch): Merge alternatives and fix attributes
    accordingly.
diff mbox

Patch

diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 164ac13..a63ec96 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -5631,7 +5631,7 @@ 
   [(set_attr "predicable" "yes")
    (set_attr "predicable_short_it" "no")
    (set_attr "length" "4")
-   (set_attr "type" "mov_imm")]
+   (set_attr "type" "alu_sreg")]
 )
 
 (define_insn "*arm_movsi_insn"
@@ -6886,7 +6886,7 @@ 
   [(set_attr "conds" "set")
    (set_attr "shift" "1")
    (set_attr "arch" "32,a,a")
-   (set_attr "type" "alus_shift_imm,alu_shift_reg,alus_shift_imm")])
+   (set_attr "type" "alus_shift_imm,alus_shift_reg,alus_shift_imm")])
 
 (define_insn "*cmpsi_shiftsi_swp"
   [(set (reg:CC_SWP CC_REGNUM)
@@ -6899,7 +6899,7 @@ 
   [(set_attr "conds" "set")
    (set_attr "shift" "1")
    (set_attr "arch" "32,a,a")
-   (set_attr "type" "alus_shift_imm,alu_shift_reg,alus_shift_imm")])
+   (set_attr "type" "alus_shift_imm,alus_shift_reg,alus_shift_imm")])
 
 (define_insn "*arm_cmpsi_negshiftsi_si"
   [(set (reg:CC_Z CC_REGNUM)
@@ -7492,10 +7492,10 @@ 
                                         (const_string "mov_imm")
                                         (const_string "mov_reg"))
                           (const_string "mvn_imm")
-                          (const_string "mov_reg")
-                          (const_string "mov_reg")
-                          (const_string "mov_reg")
-                          (const_string "mov_reg")])]
+                          (const_string "multiple")
+                          (const_string "multiple")
+                          (const_string "multiple")
+                          (const_string "multiple")])]
 )
 
 (define_insn "*movsfcc_soft_insn"
@@ -8653,7 +8653,14 @@ 
     return \"\";
   "
   [(set_attr "conds" "use")
-   (set_attr "type" "mov_reg,mov_reg,multiple")
+   (set_attr_alternative "type"
+                         [(if_then_else (match_operand 2 "const_int_operand" "")
+                                        (const_string "mov_imm")
+                                        (const_string "mov_reg"))
+                          (if_then_else (match_operand 1 "const_int_operand" "")
+                                        (const_string "mov_imm")
+                                        (const_string "mov_reg"))
+                          (const_string "multiple")])
    (set_attr "length" "4,4,8")]
 )
 
@@ -9449,8 +9456,8 @@ 
                                         (const_string "alu_imm" )
                                         (const_string "alu_sreg"))
                           (const_string "alu_imm")
-                          (const_string "alu_sreg")
-                          (const_string "alu_sreg")])]
+                          (const_string "multiple")
+                          (const_string "multiple")])]
 )
 
 (define_insn "*ifcompare_move_plus"
@@ -9487,7 +9494,13 @@ 
    sub%D4\\t%0, %2, #%n3\;mov%d4\\t%0, %1"
   [(set_attr "conds" "use")
    (set_attr "length" "4,4,8,8")
-   (set_attr "type" "alu_sreg,alu_imm,multiple,multiple")]
+   (set_attr_alternative "type"
+                         [(if_then_else (match_operand 3 "const_int_operand" "")
+                                        (const_string "alu_imm" )
+                                        (const_string "alu_sreg"))
+                          (const_string "alu_imm")
+                          (const_string "multiple")
+                          (const_string "multiple")])]
 )
 
 (define_insn "*ifcompare_arith_arith"
@@ -9582,7 +9595,11 @@ 
    %I5%d4\\t%0, %2, %3\;mov%D4\\t%0, %1"
   [(set_attr "conds" "use")
    (set_attr "length" "4,8")
-   (set_attr "type" "alu_shift_reg,multiple")]
+   (set_attr_alternative "type"
+                         [(if_then_else (match_operand 3 "const_int_operand" "")
+                                        (const_string "alu_shift_imm" )
+                                        (const_string "alu_shift_reg"))
+                          (const_string "multiple")])]
 )
 
 (define_insn "*ifcompare_move_arith"
@@ -9643,7 +9660,11 @@ 
    %I5%D4\\t%0, %2, %3\;mov%d4\\t%0, %1"
   [(set_attr "conds" "use")
    (set_attr "length" "4,8")
-   (set_attr "type" "alu_shift_reg,multiple")]
+   (set_attr_alternative "type"
+                         [(if_then_else (match_operand 3 "const_int_operand" "")
+                                        (const_string "alu_shift_imm" )
+                                        (const_string "alu_shift_reg"))
+                          (const_string "multiple")])]
 )
 
 (define_insn "*ifcompare_move_not"
@@ -9750,7 +9771,12 @@ 
   [(set_attr "conds" "use")
    (set_attr "shift" "2")
    (set_attr "length" "4,8,8")
-   (set_attr "type" "mov_shift_reg,multiple,multiple")]
+   (set_attr_alternative "type"
+                         [(if_then_else (match_operand 3 "const_int_operand" "")
+                                        (const_string "mov_shift" )
+                                        (const_string "mov_shift_reg"))
+                          (const_string "multiple")
+                          (const_string "multiple")])]
 )
 
 (define_insn "*ifcompare_move_shift"
@@ -9788,7 +9814,12 @@ 
   [(set_attr "conds" "use")
    (set_attr "shift" "2")
    (set_attr "length" "4,8,8")
-   (set_attr "type" "mov_shift_reg,multiple,multiple")]
+   (set_attr_alternative "type"
+                         [(if_then_else (match_operand 3 "const_int_operand" "")
+                                        (const_string "mov_shift" )
+                                        (const_string "mov_shift_reg"))
+                          (const_string "multiple")
+                          (const_string "multiple")])]
 )
 
 (define_insn "*ifcompare_shift_shift"
@@ -10869,7 +10900,7 @@ 
  [(set_attr "predicable" "yes")
   (set_attr "predicable_short_it" "no")
   (set_attr "length" "4")
-  (set_attr "type" "mov_imm")]
+  (set_attr "type" "alu_sreg")]
 )
 
 (define_insn "*arm_rev"
diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
index 1f68147..4f9faac 100644
--- a/gcc/config/arm/thumb2.md
+++ b/gcc/config/arm/thumb2.md
@@ -300,7 +300,7 @@ 
    ldr%?\\t%0, %1
    str%?\\t%1, %0
    str%?\\t%1, %0"
-  [(set_attr "type" "mov_reg,alu_imm,alu_imm,alu_imm,mov_imm,load1,load1,store1,store1")
+  [(set_attr "type" "mov_reg,mov_imm,mov_imm,mvn_imm,mov_imm,load1,load1,store1,store1")
    (set_attr "length" "2,4,2,4,4,4,4,4,4")
    (set_attr "predicable" "yes")
    (set_attr "predicable_short_it" "yes,no,yes,no,no,no,no,no,no")
@@ -486,12 +486,12 @@ 
 )
 
 (define_insn_and_split "*thumb2_movsicc_insn"
-  [(set (match_operand:SI 0 "s_register_operand" "=l,l,r,r,r,r,r,r,r,r,r")
+  [(set (match_operand:SI 0 "s_register_operand" "=l,l,r,r,r,r,r,r,r,r,r,r")
 	(if_then_else:SI
 	 (match_operator 3 "arm_comparison_operator"
 	  [(match_operand 4 "cc_register" "") (const_int 0)])
-	 (match_operand:SI 1 "arm_not_operand" "0 ,lPy,0 ,0,rI,K,rI,rI,K ,K,r")
-	 (match_operand:SI 2 "arm_not_operand" "lPy,0 ,rI,K,0 ,0,rI,K ,rI,K,r")))]
+	 (match_operand:SI 1 "arm_not_operand" "0 ,lPy,0 ,0,rI,K,I ,r,rI,K ,K,r")
+	 (match_operand:SI 2 "arm_not_operand" "lPy,0 ,rI,K,0 ,0,rI,I,K ,rI,K,r")))]
   "TARGET_THUMB2"
   "@
    it\\t%D3\;mov%D3\\t%0, %2
@@ -504,12 +504,14 @@ 
    #
    #
    #
+   #
    #"
    ; alt 6: ite\\t%d3\;mov%d3\\t%0, %1\;mov%D3\\t%0, %2
-   ; alt 7: ite\\t%d3\;mov%d3\\t%0, %1\;mvn%D3\\t%0, #%B2
-   ; alt 8: ite\\t%d3\;mvn%d3\\t%0, #%B1\;mov%D3\\t%0, %2
-   ; alt 9: ite\\t%d3\;mvn%d3\\t%0, #%B1\;mvn%D3\\t%0, #%B2
-   ; alt 10: ite\\t%d3\;mov%d3\\t%0, %1\;mov%D3\\t%0, %2
+   ; alt 7: ite\\t%d3\;mov%d3\\t%0, %1\;mov%D3\\t%0, %2
+   ; alt 8: ite\\t%d3\;mov%d3\\t%0, %1\;mvn%D3\\t%0, #%B2
+   ; alt 9: ite\\t%d3\;mvn%d3\\t%0, #%B1\;mov%D3\\t%0, %2
+   ; alt 10: ite\\t%d3\;mvn%d3\\t%0, #%B1\;mvn%D3\\t%0, #%B2
+   ; alt 11: ite\\t%d3\;mov%d3\\t%0, %1\;mov%D3\\t%0, %2
   "&& reload_completed"
   [(const_int 0)]
   {
@@ -540,10 +542,30 @@ 
                                                operands[2])));
     DONE;
   }
-  [(set_attr "length" "4,4,6,6,6,6,10,10,10,10,6")
-   (set_attr "enabled_for_depr_it" "yes,yes,no,no,no,no,no,no,no,no,yes")
+  [(set_attr "length" "4,4,6,6,6,6,10,8,10,10,10,6")
+   (set_attr "enabled_for_depr_it" "yes,yes,no,no,no,no,no,no,no,no,no,yes")
    (set_attr "conds" "use")
-   (set_attr "type" "multiple")]
+   (set_attr_alternative "type"
+                         [(if_then_else (match_operand 2 "const_int_operand" "")
+                                        (const_string "mov_imm")
+                                        (const_string "mov_reg"))
+                          (if_then_else (match_operand 1 "const_int_operand" "")
+                                        (const_string "mov_imm")
+                                        (const_string "mov_reg"))
+                          (if_then_else (match_operand 2 "const_int_operand" "")
+                                        (const_string "mov_imm")
+                                        (const_string "mov_reg"))
+                          (const_string "mvn_imm")
+                          (if_then_else (match_operand 1 "const_int_operand" "")
+                                        (const_string "mov_imm")
+                                        (const_string "mov_reg"))
+                          (const_string "mvn_imm")
+                          (const_string "multiple")
+                          (const_string "multiple")
+                          (const_string "multiple")
+                          (const_string "multiple")
+                          (const_string "multiple")
+                          (const_string "multiple")])]
 )
 
 (define_insn "*thumb2_movsfcc_soft_insn"
@@ -1182,7 +1204,11 @@ 
   "
   [(set_attr "predicable" "yes")
    (set_attr "length" "2")
-   (set_attr "type" "alu_sreg")]
+   (set_attr_alternative "type"
+                         [(if_then_else (match_operand 2 "const_int_operand" "")
+                                        (const_string "alu_imm")
+                                        (const_string "alu_sreg"))
+                          (const_string "alu_imm")])]
 )
 
 (define_insn "*thumb2_subsi_short"
@@ -1247,14 +1273,21 @@ 
   "
   [(set_attr "conds" "set")
    (set_attr "length" "2,2,4")
-   (set_attr "type" "alu_sreg")]
+   (set_attr_alternative "type"
+                         [(if_then_else (match_operand 2 "const_int_operand" "")
+                                        (const_string "alus_imm")
+                                        (const_string "alus_sreg"))
+                          (const_string "alus_imm")
+                          (if_then_else (match_operand 2 "const_int_operand" "")
+                                        (const_string "alus_imm")
+                                        (const_string "alus_sreg"))])]
 )
 
 (define_insn "*thumb2_addsi3_compare0_scratch"
   [(set (reg:CC_NOOV CC_REGNUM)
 	(compare:CC_NOOV
-	  (plus:SI (match_operand:SI 0 "s_register_operand" "l,l,  r,r")
-		   (match_operand:SI 1 "arm_add_operand"    "Pv,l,IL,r"))
+	  (plus:SI (match_operand:SI 0 "s_register_operand" "l,  r")
+		   (match_operand:SI 1 "arm_add_operand"    "lPv,rIL"))
 	  (const_int 0)))]
   "TARGET_THUMB2"
   "*
@@ -1271,8 +1304,10 @@ 
       return \"cmn\\t%0, %1\";
   "
   [(set_attr "conds" "set")
-   (set_attr "length" "2,2,4,4")
-   (set_attr "type" "alus_imm,alus_sreg,alus_imm,alus_sreg")]
+   (set_attr "length" "2,4")
+   (set (attr "type") (if_then_else (match_operand 2 "const_int_operand" "")
+                                    (const_string "alus_imm")
+                                    (const_string "alus_sreg")))]
 )
 
 (define_insn "*thumb2_mulsi_short"