Re: [PATCH, ARM] PR47855 Compute attr "length" for some thumb2 insns

[Carrot Wei <carrot at google dot com>]

Hi Ramana

On Thu, Mar 31, 2011 at 11:57 PM, Ramana Radhakrishnan
<ramana.radhakrishnan@linaro.org> wrote:
>>> Hi Carrot,
>>> ? ? ? ?How about adding an alternative only enabled for T2 that uses the
>>> `l'
>>> constraint and inventing new constraints for some of the constant values
>>> that are valid for 16 bit instructions since the `I' and `L' constraints
>>> have different meanings depending on whether TARGET_32BIT is valid or not
>>> ?
>>> We could then set the value of the length attribute accordingly. I don't
>>> think we can change the meaning of the I and L constraints very easily
>>> given
>>> the amount of churn that might be needed
>>>
>> You are right. Now the logic is much clearer by splitting the constraints.
>
> Sorry I wasn't too clear .
>
> What I meant was to use the "enabled" trick and enable this only when Thumb2
> is enabled in the compiler.
>
> So what you could do instead is add the alternative and then use the arch
> attribute to enable this only for Thumb2.
>
> i.e.
>
>>> ?(define_insn "*arm_cmpsi_insn"
>>> ? ?[(set (reg:CC CC_REGNUM)
>>> - ? ? ? (compare:CC (match_operand:SI 0 "s_register_operand" "r,r")
>>> - ? ? ? ? ? ? ? ? ? (match_operand:SI 1 "arm_add_operand" ? ?"rI,L")))]
>>> + ? ? ? (compare:CC (match_operand:SI 0 "s_register_operand" "l,r,r,r")
>>> + ? ? ? ? ? ? ? ? ? (match_operand:SI 1 "arm_add_operand"
>>> ?"Py,r,I,L")))]
>>> ? ?"TARGET_32BIT"
>>> ? ?"@
>>> ? ? cmp%?\\t%0, %1
>>> + ? cmp%?\\t%0, %1
>>> + ? cmp%?\\t%0, %1
>>> ? ? cmn%?\\t%0, #%n1"
>>> - ?[(set_attr "conds" "set")]
>>> + ?[(set_attr "conds" "set")
>
> ? ? ? (set_attr "arch" "t2, any")
> ? ? ? (set_attr "length" "2, 4")]
>
> Does that look any better ? That way you can now safely add options on a per
> arch basis and reduce the amount of complexity for some of these length
> calculations.
>

Thank you for the knowledge, this is the first time I see an usage of
attribute "arch".
I modified the pattern by enabling the new alternatives for thumb2 only and set
attribute "length" accordingly.

The patch was tested on arm qemu without regression, OK for install?

thanks
Carrot


ChangeLog:
2011-04-01  Wei Guozhi  <carrot@google.com>

        PR target/47855
        * config/arm/arm.md (arm_cmpsi_insn): Compute attr "length".
        (arm_cond_branch): Likewise.
        (arm_cond_branch_reversed): Likewise.
        (arm_jump): Likewise.
        (push_multi): Likewise.
        * config/arm/constraints.md (Py): New constraint.


Index: constraints.md
===================================================================
--- constraints.md	(revision 171337)
+++ constraints.md	(working copy)
@@ -31,7 +31,7 @@
 ;; The following multi-letter normal constraints have been used:
 ;; in ARM/Thumb-2 state: Da, Db, Dc, Dn, Dl, DL, Dv, Dy, Di, Dz
 ;; in Thumb-1 state: Pa, Pb, Pc, Pd
-;; in Thumb-2 state: Ps, Pt, Pu, Pv, Pw, Px
+;; in Thumb-2 state: Ps, Pt, Pu, Pv, Pw, Px, Py

 ;; The following memory constraints have been used:
 ;; in ARM/Thumb-2 state: Q, Ut, Uv, Uy, Un, Um, Us
@@ -189,6 +189,11 @@
   (and (match_code "const_int")
        (match_test "TARGET_THUMB2 && ival >= -7 && ival <= -1")))

+(define_constraint "Py"
+  "@internal In Thumb-2 state a constant in the range 0 to 255"
+  (and (match_code "const_int")
+       (match_test "TARGET_THUMB2 && ival >= 0 && ival <= 255")))
+
 (define_constraint "G"
  "In ARM/Thumb-2 state a valid FPA immediate constant."
  (and (match_code "const_double")
Index: arm.md
===================================================================
--- arm.md	(revision 171337)
+++ arm.md	(working copy)
@@ -7109,13 +7109,17 @@

 (define_insn "*arm_cmpsi_insn"
   [(set (reg:CC CC_REGNUM)
-	(compare:CC (match_operand:SI 0 "s_register_operand" "r,r")
-		    (match_operand:SI 1 "arm_add_operand"    "rI,L")))]
+	(compare:CC (match_operand:SI 0 "s_register_operand" "l,r,r,r")
+		    (match_operand:SI 1 "arm_add_operand"    "Py,r,rI,L")))]
   "TARGET_32BIT"
   "@
    cmp%?\\t%0, %1
+   cmp%?\\t%0, %1
+   cmp%?\\t%0, %1
    cmn%?\\t%0, #%n1"
-  [(set_attr "conds" "set")]
+  [(set_attr "conds" "set")
+   (set_attr "arch" "t2,t2,any,any")
+   (set_attr "length" "2,2,4,4")]
 )

 (define_insn "*cmpsi_shiftsi"
@@ -7286,7 +7290,14 @@
   return \"b%d1\\t%l0\";
   "
   [(set_attr "conds" "use")
-   (set_attr "type" "branch")]
+   (set_attr "type" "branch")
+   (set (attr "length")
+	(if_then_else
+	   (and (ne (symbol_ref "TARGET_THUMB2") (const_int 0))
+		(and (ge (minus (match_dup 0) (pc)) (const_int -250))
+		     (le (minus (match_dup 0) (pc)) (const_int 256))))
+	   (const_int 2)
+	   (const_int 4)))]
 )

 (define_insn "*arm_cond_branch_reversed"
@@ -7305,7 +7316,14 @@
   return \"b%D1\\t%l0\";
   "
   [(set_attr "conds" "use")
-   (set_attr "type" "branch")]
+   (set_attr "type" "branch")
+   (set (attr "length")
+	(if_then_else
+	   (and (ne (symbol_ref "TARGET_THUMB2") (const_int 0))
+		(and (ge (minus (match_dup 0) (pc)) (const_int -250))
+		     (le (minus (match_dup 0) (pc)) (const_int 256))))
+	   (const_int 2)
+	   (const_int 4)))]
 )



@@ -7757,7 +7775,14 @@
     return \"b%?\\t%l0\";
   }
   "
-  [(set_attr "predicable" "yes")]
+  [(set_attr "predicable" "yes")
+   (set (attr "length")
+	(if_then_else
+	   (and (ne (symbol_ref "TARGET_THUMB2") (const_int 0))
+		(and (ge (minus (match_dup 0) (pc)) (const_int -2044))
+		     (le (minus (match_dup 0) (pc)) (const_int 2048))))
+	   (const_int 2)
+	   (const_int 4)))]
 )

 (define_insn "*thumb_jump"
@@ -10256,7 +10281,29 @@

     return \"\";
   }"
-  [(set_attr "type" "store4")]
+  [(set_attr "type" "store4")
+   (set (attr "length")
+	(if_then_else
+	   (and (ne (symbol_ref "TARGET_THUMB2") (const_int 0))
+		(ne (symbol_ref "{
+		    /* Check if there are any high register (except lr)
+		       references in the list. KEEP the following iteration
+		       in sync with the template above.  */
+		    int i, regno, hi_reg;
+		    int num_saves = XVECLEN (operands[2], 0);
+		    regno = REGNO (operands[1]);
+		    hi_reg = (REGNO_REG_CLASS (regno) == HI_REGS)
+			     && (regno != LR_REGNUM);
+		    for (i = 1; i < num_saves && !hi_reg; i++)
+		      {
+			regno = REGNO (XEXP (XVECEXP (operands[2], 0, i), 0));
+			hi_reg |= (REGNO_REG_CLASS (regno) == HI_REGS)
+				  && (regno != LR_REGNUM);
+		      }
+		    !hi_reg;    }")
+		  (const_int 0)))
+	   (const_int 2)
+	   (const_int 4)))]
 )

 (define_insn "stack_tie"