[PING][PATCH, PR63742][ARM] Fix arm *movhi_insn_arch4 pattern for big-endian

["Yangfei (Felix)" <felix.yang@huawei.com>]

Hello,

    Any comments on this patch?  Thanks. 



> > >      The idea is simple: Use movw for certain const source operand
> > > instead of
> > ldrh.  And exclude the const values which cannot be handled by
> > mov/mvn/movw.
> > >      I am doing regression test for this patch.  Assuming no issue
> > > pops up,
> > OK for trunk?
> >
> > So, doesn't that makes the bug latent for architectures older than
> > armv6t2 and big endian and only fixed this in ARM state ? I'd prefer a
> > complete solution please. What about *thumb2_movhi_insn in thumb2.md ?
> >
> 
> Actually, we fix the bug by excluding the const values which cannot be handled.
> The patch still works even without the adding of "movw" here.
> The new "movw" alternative here is just an small code optimization for certain
> arch. We can handle consts by movw instead of ldrh and this better for
> performance.
> We find that this bug is not reproducible for *thumb2_movhi_insn. The reason is
> that this pattern can always move consts using "movw".
> 
> 
> > > --- gcc/ChangeLog	(revision 216838)
> > > +++ gcc/ChangeLog	(working copy)
> > > @@ -1,3 +1,12 @@
> > > +2014-11-05  Felix Yang  <felix.yang@huawei.com>
> > > +	Shanyao Chen  <chenshanyao@huawei.com>
> > > +
> >
> > I'm assuming you have copyright assignments sorted.
> 
> Yes, both my employer(Huawei) and I have signed copyright assignments with
> FSF.
> 
> 
> > > +(define_predicate "arm_movw_immediate_operand"
> > > +  (and (match_test "TARGET_32BIT && arm_arch_thumb2")
> > > +       (ior (match_code "high")
> >
> > I don't see why you need to check for "high" here ?
> >
> 
> Yeah, I double checked and found that it's not necessary here. Thanks for
> pointing this out.
> Attached please find the updated patch. Reg-tested for armeb-none-eabi.
> OK for the trunk?
> 
> 
> Index: gcc/ChangeLog
> =============================================================
> ======
> --- gcc/ChangeLog	(revision 216838)
> +++ gcc/ChangeLog	(working copy)
> @@ -1,3 +1,12 @@
> +2014-11-05  Felix Yang  <felix.yang@huawei.com>
> +	Shanyao Chen  <chenshanyao@huawei.com>
> +
> +	PR target/63742
> +	* config/arm/predicates.md (arm_hi_operand): New predicate.
> +	(arm_movw_immediate_operand): Similarly.
> +	* config/arm/arm.md (*movhi_insn_arch4): Use arm_hi_operand instead
> of
> +	general_operand and add "movw" to the output template.
> +
>  2014-10-29  Richard Sandiford  <richard.sandiford@arm.com>
> 
>  	* addresses.h, alias.c, asan.c, auto-inc-dec.c, bt-load.c, builtins.c,
> Index: gcc/config/arm/predicates.md
> =============================================================
> ======
> --- gcc/config/arm/predicates.md	(revision 216838)
> +++ gcc/config/arm/predicates.md	(working copy)
> @@ -144,6 +144,11 @@
>    (and (match_code "const_int")
>         (match_test "INTVAL (op) == 0")))
> 
> +(define_predicate "arm_movw_immediate_operand"
> +  (and (match_test "TARGET_32BIT && arm_arch_thumb2")
> +       (and (match_code "const_int")
> +	    (match_test "(INTVAL (op) & 0xffff0000) == 0"))))
> +
>  ;; Something valid on the RHS of an ARM data-processing instruction
> (define_predicate "arm_rhs_operand"
>    (ior (match_operand 0 "s_register_operand") @@ -211,6 +216,11 @@
>    (ior (match_operand 0 "arm_rhs_operand")
>         (match_operand 0 "arm_not_immediate_operand")))
> 
> +(define_predicate "arm_hi_operand"
> +  (ior (match_operand 0 "arm_rhsm_operand")
> +       (ior (match_operand 0 "arm_not_immediate_operand")
> +            (match_operand 0 "arm_movw_immediate_operand"))))
> +
>  (define_predicate "arm_di_operand"
>    (ior (match_operand 0 "s_register_operand")
>         (match_operand 0 "arm_immediate_di_operand")))
> Index: gcc/config/arm/arm.md
> =============================================================
> ======
> --- gcc/config/arm/arm.md	(revision 216838)
> +++ gcc/config/arm/arm.md	(working copy)
> @@ -6285,8 +6285,8 @@
> 
>  ;; Pattern to recognize insn generated default case above  (define_insn
> "*movhi_insn_arch4"
> -  [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,m,r")
> -	(match_operand:HI 1 "general_operand"      "rIk,K,r,mi"))]
> +  [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,r,m,r")
> +	(match_operand:HI 1 "arm_hi_operand" "rIk,K,j,r,mi"))]
>    "TARGET_ARM
>     && arm_arch4
>     && (register_operand (operands[0], HImode) @@ -6294,16 +6294,18 @@
>    "@
>     mov%?\\t%0, %1\\t%@ movhi
>     mvn%?\\t%0, #%B1\\t%@ movhi
> +   movw%?\\t%0, %1\\t%@ movhi
>     str%(h%)\\t%1, %0\\t%@ movhi
>     ldr%(h%)\\t%0, %1\\t%@ movhi"
>    [(set_attr "predicable" "yes")
> -   (set_attr "pool_range" "*,*,*,256")
> -   (set_attr "neg_pool_range" "*,*,*,244")
> +   (set_attr "pool_range" "*,*,*,*,256")
> +   (set_attr "neg_pool_range" "*,*,*,*,244")
>     (set_attr_alternative "type"
>                           [(if_then_else (match_operand 1
> "const_int_operand" "")
>                                          (const_string "mov_imm" )
>                                          (const_string "mov_reg"))
>                            (const_string "mvn_imm")
> +                          (const_string "mov_imm")
>                            (const_string "store1")
>                            (const_string "load1")])]
>  )