This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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




On 18/11/14 11:02, Yangfei (Felix) wrote:
On 06/11/14 08:35, Yangfei (Felix) wrote:
       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".

Please fix the PR number in your final commit with PR 59593.

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

I think you don't need any of these predicates.


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

Use `n' instead of `j' - movw can handle all of the numerical constants for HImode
values. And the predicate can remain general_operand.


Did you read my comment about set_attr "arch" further down in the thread ?


Look at the set_attr "arch" alternative and set this to t2 for the movw alternative. I would expect that to be enough to sort this out instead of adding all this code.




Ramana



Hello Ramana,

   We need to make sure that movw is only used for architectures which satisfy arm_arch_thumb2 as indicated in the following predicate added:

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

   I am modifying the predicate in order to fix issue for other older architectures.
   It seems we can't achieve this by simply using 'n' instead of 'j' here, right?
   Thanks.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]