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, ARM 4/7, ping1] Factor out MOVW/MOVT availability and desirability checks

From: Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>
To: Thomas Preudhomme <thomas dot preudhomme at foss dot arm dot com>, gcc-patches at gcc dot gnu dot org
Cc: Richard Earnshaw <Richard dot Earnshaw at arm dot com>, Ramana Radhakrishnan <ramana dot radhakrishnan at arm dot com>
Date: Fri, 20 May 2016 13:41:30 +0100
Subject: Re: [PATCH, ARM 4/7, ping1] Factor out MOVW/MOVT availability and desirability checks
Authentication-results: sourceware.org; auth=none
References: <003401d138a0$d3757c70$7a607550$ at foss dot arm dot com> <11620812 dot HYOIMMZIOG at e108577-lin> <573C4853 dot 8060403 at foss dot arm dot com> <4211853 dot CieZkaLpEh at e108577-lin>

Hi Thomas,

On 19/05/16 17:10, Thomas Preudhomme wrote:

On Wednesday 18 May 2016 11:47:47 Kyrill Tkachov wrote:

Hi Thomas,

Hi Kyrill,

Please find below the updated patch and associated ChangeLog entry.

*** gcc/ChangeLog ***

2016-05-18  Thomas Preud'homme  <thomas.preudhomme@arm.com>

         * config/arm/arm.h (TARGET_USE_MOVT): Check MOVT/MOVW availability
         with TARGET_HAVE_MOVT.
         (TARGET_HAVE_MOVT): Define.
         * config/arm/arm.c (const_ok_for_op): Check MOVT/MOVW
         availability with TARGET_HAVE_MOVT.
         * config/arm/arm.md (arm_movt): Use TARGET_HAVE_MOVT to check movt
         availability.
         (addsi splitter): Use TARGET_THUMB && TARGET_HAVE_MOVT rather than
         TARGET_THUMB2.
         (symbol_refs movsi splitter): Remove TARGET_32BIT check.
         (arm_movtas_ze): Use TARGET_HAVE_MOVT to check movt availability.
         * config/arm/constraints.md (define_constraint "j"): Use
         TARGET_HAVE_MOVT to check movt availability.


Please use capitalised MOVW/MOVT consistently in the ChangeLog.
Ok with a fixed ChangeLog.

Thanks,
Kyrill

diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index
1d976b36300d92d538098b3cf83c60d62ed2be1c..d199e5ebb89194fdcc962ae9653dd159a67bb7bc
100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -237,7 +237,7 @@ extern void (*arm_lang_output_object_attributes_hook)
(void);

/* Should MOVW/MOVT be used in preference to a constant pool. */

  #define TARGET_USE_MOVT \
-  (arm_arch_thumb2 \
+  (TARGET_HAVE_MOVT \
     && (arm_disable_literal_pool \
         || (!optimize_size && !current_tune->prefer_constant_pool)))

@@ -268,6 +268,9 @@ extern void (*arm_lang_output_object_attributes_hook)

(void);
  /* Nonzero if this chip supports load-acquire and store-release.  */
  #define TARGET_HAVE_LDACQ	(TARGET_ARM_ARCH >= 8 && arm_arch_notm)

+/* Nonzero if this chip provides the MOVW and MOVW instructions. */

+#define TARGET_HAVE_MOVT	(arm_arch_thumb2)
+
  /* Nonzero if integer division instructions supported.  */
  #define TARGET_IDIV	((TARGET_ARM && arm_arch_arm_hwdiv)	\
  			 || (TARGET_THUMB2 && arm_arch_thumb_hwdiv))
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index
7b95ba0b379c31ee650e714ce2198a43b1cadbac..d75a34f10d5ed22cff0a0b5d3ad433f111b059ee
100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -3897,7 +3897,7 @@ const_ok_for_op (HOST_WIDE_INT i, enum rtx_code code)
      {
      case SET:
        /* See if we can use movw.  */
-      if (arm_arch_thumb2 && (i & 0xffff0000) == 0)
+      if (TARGET_HAVE_MOVT && (i & 0xffff0000) == 0)
  	return 1;
        else
  	/* Otherwise, try mvn.  */
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index
4049f104c6d5fd8bfd8f68ecdfae6a3d34d4333f..8aa9fedf5c07e78bc7ba793b39bebcc45a4d5921
100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -5705,7 +5705,7 @@
    [(set (match_operand:SI 0 "nonimmediate_operand" "=r")
  	(lo_sum:SI (match_operand:SI 1 "nonimmediate_operand" "0")
  		   (match_operand:SI 2 "general_operand"      "i")))]
-  "arm_arch_thumb2 && arm_valid_symbolic_address_p (operands[2])"
+  "TARGET_HAVE_MOVT && arm_valid_symbolic_address_p (operands[2])"
    "movt%?\t%0, #:upper16:%c2"
    [(set_attr "predicable" "yes")
     (set_attr "predicable_short_it" "no")
@@ -5765,7 +5765,8 @@
    [(set (match_operand:SI 0 "arm_general_register_operand" "")
  	(const:SI (plus:SI (match_operand:SI 1 "general_operand" "")
  			   (match_operand:SI 2 "const_int_operand" ""))))]
-  "TARGET_THUMB2
+  "TARGET_THUMB
+   && TARGET_HAVE_MOVT
     && arm_disable_literal_pool
     && reload_completed
     && GET_CODE (operands[1]) == SYMBOL_REF"
@@ -5796,8 +5797,7 @@
  (define_split
    [(set (match_operand:SI 0 "arm_general_register_operand" "")
         (match_operand:SI 1 "general_operand" ""))]
-  "TARGET_32BIT
-   && TARGET_USE_MOVT && GET_CODE (operands[1]) == SYMBOL_REF
+  "TARGET_USE_MOVT && GET_CODE (operands[1]) == SYMBOL_REF
     && !flag_pic && !target_word_relocations
     && !arm_tls_referenced_p (operands[1])"
    [(clobber (const_int 0))]
@@ -10965,7 +10965,7 @@
                     (const_int 16)
                     (const_int 16))
          (match_operand:SI 1 "const_int_operand" ""))]
-  "arm_arch_thumb2"
+  "TARGET_HAVE_MOVT"
    "movt%?\t%0, %L1"
   [(set_attr "predicable" "yes")
    (set_attr "predicable_short_it" "no")
diff --git a/gcc/config/arm/constraints.md b/gcc/config/arm/constraints.md
index
3b71c4a527064290066348cb234c6abb8c8e2e43..4ece5f013c92adee04157b5c909e1d47c894c994
100644
--- a/gcc/config/arm/constraints.md
+++ b/gcc/config/arm/constraints.md
@@ -66,7 +66,7 @@

(define_constraint "j"

   "A constant suitable for a MOVW instruction. (ARM/Thumb-2)"
- (and (match_test "TARGET_32BIT && arm_arch_thumb2")
+ (and (match_test "TARGET_HAVE_MOVT")
        (ior (and (match_code "high")
  		(match_test "arm_valid_symbolic_address_p (XEXP (op, 0))"))
  	   (and (match_code "const_int")


Best regards,

Thomas

On 17/05/16 11:11, Thomas Preudhomme wrote:

Ping?

*** gcc/ChangeLog ***

2015-11-09  Thomas Preud'homme  <thomas.preudhomme@arm.com>

          * config/arm/arm.h (TARGET_USE_MOVT): Check MOVT/MOVW
          availability
          with TARGET_HAVE_MOVT.
          (TARGET_HAVE_MOVT): Define.
          * config/arm/arm.c (const_ok_for_op): Check MOVT/MOVW
          availability with TARGET_HAVE_MOVT.
          * config/arm/arm.md (arm_movt): Use TARGET_HAVE_MOVT to check
          movt
          availability.
          (addsi splitter): Use TARGET_USE_MOVT to check whether to use
          movt + movw.
          (symbol_refs movsi splitter): Remove TARGET_32BIT check.
          (arm_movtas_ze): Use TARGET_HAVE_MOVT to check movt availability.
          * config/arm/constraints.md (define_constraint "j"): Use
          TARGET_HAVE_MOVT to check movt availability.

diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index
1d976b36300d92d538098b3cf83c60d62ed2be1c..47216b4a1959ccdb18e329db411bf7f9
41e67163 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -237,7 +237,7 @@ extern void (*arm_lang_output_object_attributes_hook)
(void);

   /* Should MOVW/MOVT be used in preference to a constant pool.  */
   #define TARGET_USE_MOVT \

-  (arm_arch_thumb2 \
+  (TARGET_HAVE_MOVT \

      && (arm_disable_literal_pool \

|| (!optimize_size && !current_tune->prefer_constant_pool)))


@@ -268,6 +268,9 @@ extern void (*arm_lang_output_object_attributes_hook)
(void);

   /* Nonzero if this chip supports load-acquire and store-release.  */
   #define TARGET_HAVE_LDACQ	(TARGET_ARM_ARCH >= 8 && arm_arch_notm)

+/* Nonzero if this chip provides the movw and movt instructions.  */
+#define TARGET_HAVE_MOVT	(arm_arch_thumb2)
+

   /* Nonzero if integer division instructions supported.  */
   #define TARGET_IDIV	((TARGET_ARM && arm_arch_arm_hwdiv)	\

|| (TARGET_THUMB2 && arm_arch_thumb_hwdiv))


diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index
7b95ba0b379c31ee650e714ce2198a43b1cadbac..d75a34f10d5ed22cff0a0b5d3ad433f1
11b059ee 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -3897,7 +3897,7 @@ const_ok_for_op (HOST_WIDE_INT i, enum rtx_code
code)

       {

case SET:

         /* See if we can use movw.  */

-      if (arm_arch_thumb2 && (i & 0xffff0000) == 0)
+      if (TARGET_HAVE_MOVT && (i & 0xffff0000) == 0)

   	return 1;
   	
         else
   	
   	/* Otherwise, try mvn.  */

diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index
4049f104c6d5fd8bfd8f68ecdfae6a3d34d4333f..0333394423477acb8d9223fd06c17e82
bfd0a94d 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -5705,7 +5705,7 @@

     [(set (match_operand:SI 0 "nonimmediate_operand" "=r")
   	
   	(lo_sum:SI (match_operand:SI 1 "nonimmediate_operand" "0")
   	
   		   (match_operand:SI 2 "general_operand"      "i")))]

-  "arm_arch_thumb2 && arm_valid_symbolic_address_p (operands[2])"
+  "TARGET_HAVE_MOVT && arm_valid_symbolic_address_p (operands[2])"

     "movt%?\t%0, #:upper16:%c2"
     [(set_attr "predicable" "yes")

(set_attr "predicable_short_it" "no")


@@ -5765,8 +5765,7 @@

     [(set (match_operand:SI 0 "arm_general_register_operand" "")
   	
   	(const:SI (plus:SI (match_operand:SI 1 "general_operand" "")
   	
   			   (match_operand:SI 2 "const_int_operand" ""))))]

-  "TARGET_THUMB2
-   && arm_disable_literal_pool
+  "TARGET_USE_MOVT

This is not an equivalent change.
First, TARGET_THUMB2 and arm_arch_thumb2 are not exactly the same.
TARGET_THUMB2 means that the selected architecture supports Thumb2 AND
the user is compiling for Thumb2. arm_arch_thumb2 on the other hand means
that the selected architecture supports Thumb2, but will be set even when
when compiling for -marm (for example -march=armv7-a -marm).

In this case we want the pattern to apply only when actually targeting
Thumb2.

Second,

TARGET_USE_MOVT is not just TARGET_THUMB2 && arm_disable_literal_pool.
(With this patch) it's defined as:
#define TARGET_USE_MOVT \
    (TARGET_HAVE_MOVT \
     && (arm_disable_literal_pool \

         || (!optimize_size && !current_tune->prefer_constant_pool)))

So, if you want to enable this pattern for ARMv8-M in the next patch I think
what you want is to replace TARGET_THUMB2 by TARGET_THUMB &&
TARGET_HAVE_MOVT

Kyrill

      && reload_completed
      && GET_CODE (operands[1]) == SYMBOL_REF"

[(clobber (const_int 0))]


@@ -5796,8 +5795,7 @@

   (define_split

[(set (match_operand:SI 0 "arm_general_register_operand" "")(match_operand:SI 1 "general_operand" ""))]


-  "TARGET_32BIT
-   && TARGET_USE_MOVT && GET_CODE (operands[1]) == SYMBOL_REF
+  "TARGET_USE_MOVT && GET_CODE (operands[1]) == SYMBOL_REF

      && !flag_pic && !target_word_relocations
      && !arm_tls_referenced_p (operands[1])"

[(clobber (const_int 0))]


@@ -10965,7 +10963,7 @@

                      (const_int 16)
                      (const_int 16))

(match_operand:SI 1 "const_int_operand" ""))]


-  "arm_arch_thumb2"
+  "TARGET_HAVE_MOVT"

     "movt%?\t%0, %L1"

[(set_attr "predicable" "yes")(set_attr "predicable_short_it" "no")


diff --git a/gcc/config/arm/constraints.md b/gcc/config/arm/constraints.md
index
3b71c4a527064290066348cb234c6abb8c8e2e43..4ece5f013c92adee04157b5c909e1d47
c894c994 100644
--- a/gcc/config/arm/constraints.md
+++ b/gcc/config/arm/constraints.md
@@ -66,7 +66,7 @@

   (define_constraint "j"

"A constant suitable for a MOVW instruction. (ARM/Thumb-2)"


- (and (match_test "TARGET_32BIT && arm_arch_thumb2")
+ (and (match_test "TARGET_HAVE_MOVT")

         (ior (and (match_code "high")
   		
   		(match_test "arm_valid_symbolic_address_p (XEXP (op, 0))"))
   		
   	   (and (match_code "const_int")

Best regards,

Thomas

On Thursday 17 December 2015 15:59:13 Thomas Preud'homme wrote:

Hi,

This patch is part of a patch series to add support for ARMv8-M[1] to
GCC.
This specific patch factors out the checks for MOVW/MOVT availability and
whether to use it. To this end, the new macro TARGET_HAVE_MOVT is
introduced and code is modified to use it or the existing TARGET_USE_MOVT
as needed.

[1] For a quick overview of ARMv8-M please refer to the initial cover
letter.

ChangeLog entry is as follows:


*** gcc/ChangeLog ***

2015-11-09  Thomas Preud'homme  <thomas.preudhomme@arm.com>

          * config/arm/arm.h (TARGET_USE_MOVT): Check MOVT/MOVW
          availability
          with TARGET_HAVE_MOVT.
          (TARGET_HAVE_MOVT): Define.
          * config/arm/arm.c (const_ok_for_op): Check MOVT/MOVW
          availability with TARGET_HAVE_MOVT.
          * config/arm/arm.md (arm_movt): Use TARGET_HAVE_MOVT to check
          movt
          availability.
          (addsi splitter): Use TARGET_USE_MOVT to check whether to use
          movt + movw.
          (symbol_refs movsi splitter): Remove TARGET_32BIT check.
          (arm_movtas_ze): Use TARGET_HAVE_MOVT to check movt
          availability.
          * config/arm/constraints.md (define_constraint "j"): Use
          TARGET_HAVE_MOVT to check movt availability.

diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index fed3205..1831d01 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -233,7 +233,7 @@ extern void
(*arm_lang_output_object_attributes_hook)(void);

   /* Should MOVW/MOVT be used in preference to a constant pool.  */
   #define TARGET_USE_MOVT \

-  (arm_arch_thumb2 \
+  (TARGET_HAVE_MOVT \

      && (arm_disable_literal_pool \

|| (!optimize_size && !current_tune->prefer_constant_pool)))


@@ -268,6 +268,9 @@ extern void
(*arm_lang_output_object_attributes_hook)(void); /* Nonzero if this chip
supports load-acquire and store-release.  */ #define
TARGET_HAVE_LDACQ	(TARGET_ARM_ARCH >= 8)

+/* Nonzero if this chip provides the movw and movt instructions.  */
+#define TARGET_HAVE_MOVT	(arm_arch_thumb2)
+

   /* Nonzero if integer division instructions supported.  */
   #define TARGET_IDIV	((TARGET_ARM && arm_arch_arm_hwdiv)	\

|| (TARGET_THUMB2 && arm_arch_thumb_hwdiv))


diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 62287bc..ec5197a 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -3851,7 +3851,7 @@ const_ok_for_op (HOST_WIDE_INT i, enum rtx_code
code)

       {

case SET:

         /* See if we can use movw.  */

-      if (arm_arch_thumb2 && (i & 0xffff0000) == 0)
+      if (TARGET_HAVE_MOVT && (i & 0xffff0000) == 0)

   	return 1;
   	
         else
   	
   	/* Otherwise, try mvn.  */

diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 8ebb1bf..78dafa0 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -5736,7 +5736,7 @@

     [(set (match_operand:SI 0 "nonimmediate_operand" "=r")
   	
   	(lo_sum:SI (match_operand:SI 1 "nonimmediate_operand" "0")
   	
   		   (match_operand:SI 2 "general_operand"      "i")))]

-  "arm_arch_thumb2 && arm_valid_symbolic_address_p (operands[2])"
+  "TARGET_HAVE_MOVT && arm_valid_symbolic_address_p (operands[2])"

     "movt%?\t%0, #:upper16:%c2"
     [(set_attr "predicable" "yes")

(set_attr "predicable_short_it" "no")


@@ -5796,8 +5796,7 @@

     [(set (match_operand:SI 0 "arm_general_register_operand" "")
   	
   	(const:SI (plus:SI (match_operand:SI 1 "general_operand" "")
   	
   			   (match_operand:SI 2 "const_int_operand" ""))))]

-  "TARGET_THUMB2
-   && arm_disable_literal_pool
+  "TARGET_USE_MOVT

      && reload_completed
      && GET_CODE (operands[1]) == SYMBOL_REF"

[(clobber (const_int 0))]


@@ -5827,8 +5826,7 @@

   (define_split

[(set (match_operand:SI 0 "arm_general_register_operand" "")(match_operand:SI 1 "general_operand" ""))]


-  "TARGET_32BIT
-   && TARGET_USE_MOVT && GET_CODE (operands[1]) == SYMBOL_REF
+  "TARGET_USE_MOVT && GET_CODE (operands[1]) == SYMBOL_REF

      && !flag_pic && !target_word_relocations
      && !arm_tls_referenced_p (operands[1])"

[(clobber (const_int 0))]


@@ -11030,7 +11028,7 @@

                      (const_int 16)
                      (const_int 16))

(match_operand:SI 1 "const_int_operand" ""))]


-  "arm_arch_thumb2"
+  "TARGET_HAVE_MOVT"

     "movt%?\t%0, %L1"

[(set_attr "predicable" "yes")(set_attr "predicable_short_it" "no")


diff --git a/gcc/config/arm/constraints.md
b/gcc/config/arm/constraints.md
index d01a918..838e031 100644
--- a/gcc/config/arm/constraints.md
+++ b/gcc/config/arm/constraints.md
@@ -67,7 +67,7 @@

   (define_constraint "j"

"A constant suitable for a MOVW instruction. (ARM/Thumb-2)"


- (and (match_test "TARGET_32BIT && arm_arch_thumb2")
+ (and (match_test "TARGET_HAVE_MOVT")

         (ior (and (match_code "high")
   		
   		(match_test "arm_valid_symbolic_address_p (XEXP (op, 0))"))
   		
   	   (and (match_code "const_int")

Testing:

* Toolchain was built successfully with the following multilib list:
armv6-m,armv7-m,armv7e-m,cortex-m7. The code generation for crtbegin.o,
crtend.o, crti.o, crtn.o, libgcc.a, libgcov.a, libc.a, libg.a,
libgloss-linux.a, libm.a, libnosys.a, librdimon.a, librdpmon.a,
libstdc++.a
and libsupc++.a is unchanged for all these targets.

* GCC also showed no testsuite regression when targeting ARMv6-M and
ARMv7-M (compared to without the patch) * GCC was bootstrapped
successfully targeting Thumb-1 and targeting Thumb-2

Is this ok for stage3?

Best regards,

Thomas

References:
- Re: [PATCH, ARM 4/7, ping1] Factor out MOVW/MOVT availability and desirability checks
  - From: Thomas Preudhomme
- Re: [PATCH, ARM 4/7, ping1] Factor out MOVW/MOVT availability and desirability checks
  - From: Kyrill Tkachov
- Re: [PATCH, ARM 4/7, ping1] Factor out MOVW/MOVT availability and desirability checks
  - From: Thomas Preudhomme

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