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

[Ramana Radhakrishnan]

On 19/11/14 09:29, Yangfei (Felix) wrote:
Sorry for missing the point.  It seems to me that 't2' here will conflict with
condition of the pattern *movhi_insn_arch4:
    "TARGET_ARM
     && arm_arch4
     && (register_operand (operands[0], HImode)
         || register_operand (operands[1], HImode))"

#define TARGET_ARM                      (! TARGET_THUMB)
/* 32-bit Thumb-2 code.  */
#define TARGET_THUMB2                   (TARGET_THUMB &&
arm_arch_thumb2)


Bah, Indeed ! - I misremembered the t2 there, my mistake.

Yes you are right there, but what I'd like you to do is to use that mechanism
rather than putting all this logic in the predicate.

So, I'd prefer you to add a v6t2 to the values for the "arch" attribute, don't 
forget
to update the comments above.

and in arch_enabled you need to enforce this with

   (and (eq_attr "arch" "v6t2")
        (match_test "TARGET_32BIT && arm_arch6 && arm_arch_thumb2"))
         (const_string "yes")

And in the pattern use v6t2 ...

arm_arch_thumb2 implies that this is at the architecture level of v6t2.
Therefore TARGET_ARM && arm_arch_thumb2 implies ARM state.


Hi Ramana,
     Thank you for your suggestions.  I rebased the patch on the latest trunk 
and updated it accordingly.
     As this patch will not work for architectures older than armv6t2,  I also 
prefer Thomas's patch to fix for them.
     I am currently performing test for this patch.  Assuming no issues pops 
up, OK for the trunk?
     And is it necessary to backport this patch to the 4.8 & 4.9 branches?



This is OK for trunk if no regressions - please let it bake for a few 
days on trunk to let auto-testers catch up. This is OK for 4.8 / 4.9 
after that and please test your backport.

regards
Ramana




Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog       (revision 217717)
+++ gcc/ChangeLog       (working copy)
@@ -1,3 +1,11 @@
+2014-11-19  Felix Yang  <felix.y...@huawei.com>
+           Shanyao Chen  <chenshan...@huawei.com>
+
+       PR target/59593
+       * config/arm/arm.md (define_attr "arch"): Add v6t2.
+       (define_attr "arch_enabled"): Add test for the above.
+       (*movhi_insn_arch4): Add new alternative.
+
  2014-11-18  Felix Yang  <felix.y...@huawei.com>

        * config/aarch64/aarch64.c (doloop_end): New pattern.
Index: gcc/config/arm/arm.md
===================================================================
--- gcc/config/arm/arm.md       (revision 217717)
+++ gcc/config/arm/arm.md       (working copy)
@@ -125,9 +125,10 @@
  ; This can be "a" for ARM, "t" for either of the Thumbs, "32" for
  ; TARGET_32BIT, "t1" or "t2" to specify a specific Thumb mode.  "v6"
  ; for ARM or Thumb-2 with arm_arch6, and nov6 for ARM without
-; arm_arch6.  This attribute is used to compute attribute "enabled",
-; use type "any" to enable an alternative in all cases.
-(define_attr "arch" 
"any,a,t,32,t1,t2,v6,nov6,neon_for_64bits,avoid_neon_for_64bits,iwmmxt,iwmmxt2,armv6_or_vfpv3"
+; arm_arch6.  "v6t2" for Thumb-2 with arm_arch6.  This attribute is
+; used to compute attribute "enabled", use type "any" to enable an
+; alternative in all cases.
+(define_attr "arch" 
"any,a,t,32,t1,t2,v6,nov6,v6t2,neon_for_64bits,avoid_neon_for_64bits,iwmmxt,iwmmxt2,armv6_or_vfpv3"
    (const_string "any"))

  (define_attr "arch_enabled" "no,yes"
@@ -162,6 +163,10 @@
              (match_test "TARGET_32BIT && !arm_arch6"))
         (const_string "yes")

+        (and (eq_attr "arch" "v6t2")
+             (match_test "TARGET_32BIT && arm_arch6 && arm_arch_thumb2"))
+        (const_string "yes")
+
         (and (eq_attr "arch" "avoid_neon_for_64bits")
              (match_test "TARGET_NEON")
              (not (match_test "TARGET_PREFER_NEON_64BITS")))
@@ -6288,8 +6293,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 "general_operand"      "rIk,K,n,r,mi"))]
    "TARGET_ARM
     && arm_arch4
     && (register_operand (operands[0], HImode)
@@ -6297,16 +6302,19 @@
    "@
     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 "arch" "*,*,v6t2,*,*")
     (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")])]
  )