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

Ramana Radhakrishnan ramana.radhakrishnan@arm.com
Wed Nov 5 11:01:00 GMT 2014



On 05/11/14 07:09, Yangfei (Felix) wrote:
> Hi,
>
>      This patch fixes PR63742 by improving arm *movhi_insn_arch4 pattern to make it works under big-endian.
>      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 ?

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

I'm assuming you have copyright assignments sorted.


> +	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,12 @@
>     (and (match_code "const_int")
>          (match_test "INTVAL (op) == 0")))
>
> +(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 ?

> +            (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 +217,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")])]
>   )
>

Ramana



More information about the Gcc-patches mailing list