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

["Yangfei (Felix)" <felix dot yang at huawei dot com>]

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


Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog	(revision 217717)
+++ gcc/ChangeLog	(working copy)
@@ -1,3 +1,11 @@
+2014-11-19  Felix Yang  <felix.yang@huawei.com>
+	    Shanyao Chen  <chenshanyao@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.yang@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")])]
 )

Attachment: arm-patch-v3.diff
Description: arm-patch-v3.diff