[Patch][ARM]Don't put volatile memory access in IT block for cortex-m7

[Patch][ARM]Don't put volatile memory access in IT block for cortex-m7

[Terry Guo]

[-- Attachment #1: Type: text/plain, Size: 634 bytes --]

Hi there,

This patch intends to prevent gcc from putting volatile memory access into
IT block for target like cortex-m7.

gcc/ChangeLog:

2015-02-12  Terry Guo  <terry.guo@arm.com>

	* config/arm/arm.c (arm_tune_cortex_m7): New global variable.
	* config/arm/arm.h (TARGET_NO_VOLATILE_CE): New macro.
                (arm_tune_cortex_m7): Declare new global variable.
	* config/arm/arm.md (arm_comparison_operator): Disabled if not allow
                 volatile memory access in IT block.

gcc/testsuite/ChangeLog:

2015-02-12  Terry Guo  <terry.guo@arm.com>

	* gcc.target/arm/cortex-m7-it-volatile.c: New test.

[-- Attachment #2: no-volatile-in-it-v4.txt --]
[-- Type: text/plain, Size: 2946 bytes --]

diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 297dfe1..d6b854d 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -290,6 +290,9 @@ extern void (*arm_lang_output_object_attributes_hook)(void);
 
 #define TARGET_CRC32			(arm_arch_crc)
 
+/* Targets that don't support accessing volatile memory inside IT block.  */
+#define TARGET_NO_VOLATILE_CE		(arm_tune_cortex_m7)
+
 /* The following two macros concern the ability to execute coprocessor
    instructions for VFPv3 or NEON.  TARGET_VFP3/TARGET_VFPD32 are currently
    only ever tested when we know we are generating for VFP hardware; we need
@@ -552,6 +555,9 @@ extern int arm_tune_wbuf;
 /* Nonzero if tuning for Cortex-A9.  */
 extern int arm_tune_cortex_a9;
 
+/* Nonzero if tuning for Cortex-M7.  */
+extern int arm_tune_cortex_m7;
+
 /* Nonzero if we should define __THUMB_INTERWORK__ in the
    preprocessor.
    XXX This is a bit of a hack, it's intended to help work around
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 7bf5b4d..081ccec 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -846,6 +846,9 @@ int arm_tune_wbuf = 0;
 /* Nonzero if tuning for Cortex-A9.  */
 int arm_tune_cortex_a9 = 0;
 
+/* Nonzero if tuning for Cortex-M7.  */
+int arm_tune_cortex_m7 = 0;
+
 /* Nonzero if generating Thumb instructions.  */
 int thumb_code = 0;
 
@@ -2859,7 +2862,8 @@ arm_option_override (void)
   arm_arch_iwmmxt2 = (insn_flags & FL_IWMMXT2) != 0;
   arm_arch_thumb_hwdiv = (insn_flags & FL_THUMB_DIV) != 0;
   arm_arch_arm_hwdiv = (insn_flags & FL_ARM_DIV) != 0;
-  arm_tune_cortex_a9 = (arm_tune == cortexa9) != 0;
+  arm_tune_cortex_a9 = (arm_tune == cortexa9);
+  arm_tune_cortex_m7 = (arm_tune == cortexm7);
   arm_arch_crc = (insn_flags & FL_CRC32) != 0;
   arm_m_profile_small_mul = (insn_flags & FL_SMALLMUL) != 0;
   if (arm_restrict_it == 2)
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index c13e9b2..164ac13 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -10755,7 +10755,8 @@
   [(match_operator 0 "arm_comparison_operator"
     [(match_operand 1 "cc_register" "")
      (const_int 0)])]
-  "TARGET_32BIT"
+  "TARGET_32BIT
+   && (!TARGET_NO_VOLATILE_CE || !volatile_refs_p (PATTERN (insn)))"
   ""
 [(set_attr "predicated" "yes")]
 )
diff --git a/gcc/testsuite/gcc.target/arm/cortex-m7-it-volatile.c b/gcc/testsuite/gcc.target/arm/cortex-m7-it-volatile.c
new file mode 100644
index 0000000..206afdb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/cortex-m7-it-volatile.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_thumb2_ok } */
+/* { dg-options "-Os -mthumb -mcpu=cortex-m7" } */
+
+int
+foo (int a, int b, volatile int *c, volatile int *d)
+{
+  if (a > b)
+    return c[0];
+  else
+    return d[0];
+}
+
+/* { dg-final { scan-assembler-not "ldrgt" } } */

Re: [Patch][ARM]Don't put volatile memory access in IT block for cortex-m7

[Richard Earnshaw]

On 12/02/15 11:12, Terry Guo wrote:
> Hi there,
> 
> This patch intends to prevent gcc from putting volatile memory access into
> IT block for target like cortex-m7.
> 
> gcc/ChangeLog:
> 
> 2015-02-12  Terry Guo  <terry.guo@arm.com>
> 
> 	* config/arm/arm.c (arm_tune_cortex_m7): New global variable.
> 	* config/arm/arm.h (TARGET_NO_VOLATILE_CE): New macro.
>                 (arm_tune_cortex_m7): Declare new global variable.
> 	* config/arm/arm.md (arm_comparison_operator): Disabled if not allow
>                  volatile memory access in IT block.
> 
> gcc/testsuite/ChangeLog:
> 
> 2015-02-12  Terry Guo  <terry.guo@arm.com>
> 
> 	* gcc.target/arm/cortex-m7-it-volatile.c: New test.
> 

Not ok.

+/* Targets that don't support accessing volatile memory inside IT
block.  */
+#define TARGET_NO_VOLATILE_CE		(arm_tune_cortex_m7)

Please don't create feature bits that explicitly test for a particular
target.  Instead, define generic 'features' and then arrange for either
the architecture tables, or tuning tables (as appropriate) to enable
that feature.

See how arm_arch_arm_hwdiv is defined for how to do this.

R.

RE: [Patch][ARM]Don't put volatile memory access in IT block for cortex-m7

[Terry Guo]

[-- Attachment #1: Type: text/plain, Size: 2481 bytes --]



> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Richard Earnshaw
> Sent: Wednesday, February 18, 2015 2:45 AM
> To: Terry Guo; gcc-patches@gcc.gnu.org
> Cc: Richard Earnshaw; Ramana Radhakrishnan
> Subject: Re: [Patch][ARM]Don't put volatile memory access in IT block for
> cortex-m7
> 
> On 12/02/15 11:12, Terry Guo wrote:
> > Hi there,
> >
> > This patch intends to prevent gcc from putting volatile memory access
> > into IT block for target like cortex-m7.
> >
> > gcc/ChangeLog:
> >
> > 2015-02-12  Terry Guo  <terry.guo@arm.com>
> >
> > 	* config/arm/arm.c (arm_tune_cortex_m7): New global variable.
> > 	* config/arm/arm.h (TARGET_NO_VOLATILE_CE): New macro.
> >                 (arm_tune_cortex_m7): Declare new global variable.
> > 	* config/arm/arm.md (arm_comparison_operator): Disabled if not
> allow
> >                  volatile memory access in IT block.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2015-02-12  Terry Guo  <terry.guo@arm.com>
> >
> > 	* gcc.target/arm/cortex-m7-it-volatile.c: New test.
> >
> 
> Not ok.
> 
> +/* Targets that don't support accessing volatile memory inside IT
> block.  */
> +#define TARGET_NO_VOLATILE_CE		(arm_tune_cortex_m7)
> 
> Please don't create feature bits that explicitly test for a particular target.
> Instead, define generic 'features' and then arrange for either the
> architecture tables, or tuning tables (as appropriate) to enable that feature.
> 
> See how arm_arch_arm_hwdiv is defined for how to do this.
> 
> R.
> 

Thanks Richard.  Patch is updated per your suggestion. Is this one OK for current stage and 4.8/4.9?

BR,
Terry

gcc/testsuite/ChangeLog:

2015-02-25  Terry Guo  <terry.guo@arm.com>

    * gcc.target/arm/no-volatile-in-it.c: New test.


gcc/ChangeLog:

2015-02-25  Terry Guo  <terry.guo@arm.com>

    * config/arm/arm-cores.def (cortex-m7): Add flag FL_NO_VOLATILE_CE.
    * config/arm/arm-protos.h (FL_NO_VOLATILE_CE): New flag.
    (arm_arch_no_volatile_ce): Declare new global variable.
    * config/arm/arm.c (arm_arch_no_volatile_ce): Define new global variable.
    (arm_option_override): Assign value to arm_arch_no_volatile_ce.
    * config/arm/arm.h (arm_arch_no_volatile_ce): Declare it.
    (TARGET_NO_VOLATILE_CE): New macro.
    * config/arm/arm.md (arm_comparison_operator): Disabled if not allow
    volatile memory access in IT block

[-- Attachment #2: no-volatile-in-it-v5.txt --]
[-- Type: text/plain, Size: 4855 bytes --]

diff --git a/gcc/config/arm/arm-cores.def b/gcc/config/arm/arm-cores.def
index d7e730d..b22ea7f 100644
--- a/gcc/config/arm/arm-cores.def
+++ b/gcc/config/arm/arm-cores.def
@@ -155,7 +155,7 @@ ARM_CORE("cortex-r4",		cortexr4, cortexr4,		7R,  FL_LDSCHED, cortex)
 ARM_CORE("cortex-r4f",		cortexr4f, cortexr4f,		7R,  FL_LDSCHED, cortex)
 ARM_CORE("cortex-r5",		cortexr5, cortexr5,		7R,  FL_LDSCHED | FL_ARM_DIV, cortex)
 ARM_CORE("cortex-r7",		cortexr7, cortexr7,		7R,  FL_LDSCHED | FL_ARM_DIV, cortex)
-ARM_CORE("cortex-m7",		cortexm7, cortexm7,		7EM, FL_LDSCHED, cortex_m7)
+ARM_CORE("cortex-m7",		cortexm7, cortexm7,		7EM, FL_LDSCHED | FL_NO_VOLATILE_CE, cortex_m7)
 ARM_CORE("cortex-m4",		cortexm4, cortexm4,		7EM, FL_LDSCHED, v7m)
 ARM_CORE("cortex-m3",		cortexm3, cortexm3,		7M,  FL_LDSCHED, v7m)
 ARM_CORE("marvell-pj4",		marvell_pj4, marvell_pj4,	7A,  FL_LDSCHED, 9e)
diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index 307babb..28ffe52 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -360,6 +360,7 @@ extern bool arm_is_constant_pool_ref (rtx);
 #define FL_CRC32      (1 << 25)	      /* ARMv8 CRC32 instructions.  */
 
 #define FL_SMALLMUL   (1 << 26)       /* Small multiply supported.  */
+#define FL_NO_VOLATILE_CE   (1 << 27) /* No volatile memory in IT block.  */
 
 #define FL_IWMMXT     (1 << 29)	      /* XScale v2 or "Intel Wireless MMX technology".  */
 #define FL_IWMMXT2    (1 << 30)       /* "Intel Wireless MMX2 technology".  */
@@ -482,6 +483,9 @@ extern int arm_arch_thumb2;
 extern int arm_arch_arm_hwdiv;
 extern int arm_arch_thumb_hwdiv;
 
+/* Nonzero if chip disallows volatile memory access in IT block.  */
+extern int arm_arch_no_volatile_ce;
+
 /* Nonzero if we should use Neon to handle 64-bits operations rather
    than core registers.  */
 extern int prefer_neon_for_64bits;
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 297dfe1..8c10ea3 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -383,6 +383,9 @@ extern void (*arm_lang_output_object_attributes_hook)(void);
 #define TARGET_IDIV		((TARGET_ARM && arm_arch_arm_hwdiv) \
 				 || (TARGET_THUMB2 && arm_arch_thumb_hwdiv))
 
+/* Nonzero if disallow volatile memory access in IT block.  */
+#define TARGET_NO_VOLATILE_CE		(arm_arch_no_volatile_ce)
+
 /* Should NEON be used for 64-bits bitops.  */
 #define TARGET_PREFER_NEON_64BITS (prefer_neon_for_64bits)
 
@@ -568,6 +571,9 @@ extern int arm_arch_arm_hwdiv;
 /* Nonzero if chip supports integer division instruction in Thumb mode.  */
 extern int arm_arch_thumb_hwdiv;
 
+/* Nonzero if chip disallows volatile memory access in IT block.  */
+extern int arm_arch_no_volatile_ce;
+
 /* Nonzero if we should use Neon to handle 64-bits operations rather
    than core registers.  */
 extern int prefer_neon_for_64bits;
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 7bf5b4d..f7063bc 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -866,6 +866,9 @@ int arm_arch_thumb2;
 int arm_arch_arm_hwdiv;
 int arm_arch_thumb_hwdiv;
 
+/* Nonzero if chip disallows volatile memory access in IT block.  */
+int arm_arch_no_volatile_ce;
+
 /* Nonzero if we should use Neon to handle 64-bits operations rather
    than core registers.  */
 int prefer_neon_for_64bits = 0;
@@ -2859,6 +2862,7 @@ arm_option_override (void)
   arm_arch_iwmmxt2 = (insn_flags & FL_IWMMXT2) != 0;
   arm_arch_thumb_hwdiv = (insn_flags & FL_THUMB_DIV) != 0;
   arm_arch_arm_hwdiv = (insn_flags & FL_ARM_DIV) != 0;
+  arm_arch_no_volatile_ce = (insn_flags & FL_NO_VOLATILE_CE) != 0;
   arm_tune_cortex_a9 = (arm_tune == cortexa9) != 0;
   arm_arch_crc = (insn_flags & FL_CRC32) != 0;
   arm_m_profile_small_mul = (insn_flags & FL_SMALLMUL) != 0;
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index c13e9b2..164ac13 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -10755,7 +10755,8 @@
   [(match_operator 0 "arm_comparison_operator"
     [(match_operand 1 "cc_register" "")
      (const_int 0)])]
-  "TARGET_32BIT"
+  "TARGET_32BIT
+   && (!TARGET_NO_VOLATILE_CE || !volatile_refs_p (PATTERN (insn)))"
   ""
 [(set_attr "predicated" "yes")]
 )
diff --git a/gcc/testsuite/gcc.target/arm/no-volatile-in-it.c b/gcc/testsuite/gcc.target/arm/no-volatile-in-it.c
new file mode 100644
index 0000000..206afdb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/no-volatile-in-it.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_thumb2_ok } */
+/* { dg-options "-Os -mthumb -mcpu=cortex-m7" } */
+
+int
+foo (int a, int b, volatile int *c, volatile int *d)
+{
+  if (a > b)
+    return c[0];
+  else
+    return d[0];
+}
+
+/* { dg-final { scan-assembler-not "ldrgt" } } */

Re: [Patch][ARM]Don't put volatile memory access in IT block for cortex-m7

[Richard Earnshaw]

On 25/02/15 10:42, Terry Guo wrote:
>
>
>> -----Original Message-----
>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
>> owner@gcc.gnu.org] On Behalf Of Richard Earnshaw
>> Sent: Wednesday, February 18, 2015 2:45 AM
>> To: Terry Guo; gcc-patches@gcc.gnu.org
>> Cc: Richard Earnshaw; Ramana Radhakrishnan
>> Subject: Re: [Patch][ARM]Don't put volatile memory access in IT block for
>> cortex-m7
>>
>> On 12/02/15 11:12, Terry Guo wrote:
>>> Hi there,
>>>
>>> This patch intends to prevent gcc from putting volatile memory access
>>> into IT block for target like cortex-m7.
>>>
>>> gcc/ChangeLog:
>>>
>>> 2015-02-12  Terry Guo  <terry.guo@arm.com>
>>>
>>> 	* config/arm/arm.c (arm_tune_cortex_m7): New global variable.
>>> 	* config/arm/arm.h (TARGET_NO_VOLATILE_CE): New macro.
>>>                  (arm_tune_cortex_m7): Declare new global variable.
>>> 	* config/arm/arm.md (arm_comparison_operator): Disabled if not
>> allow
>>>                   volatile memory access in IT block.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2015-02-12  Terry Guo  <terry.guo@arm.com>
>>>
>>> 	* gcc.target/arm/cortex-m7-it-volatile.c: New test.
>>>
>>
>> Not ok.
>>
>> +/* Targets that don't support accessing volatile memory inside IT
>> block.  */
>> +#define TARGET_NO_VOLATILE_CE		(arm_tune_cortex_m7)
>>
>> Please don't create feature bits that explicitly test for a particular target.
>> Instead, define generic 'features' and then arrange for either the
>> architecture tables, or tuning tables (as appropriate) to enable that feature.
>>
>> See how arm_arch_arm_hwdiv is defined for how to do this.
>>
>> R.
>>
>
> Thanks Richard.  Patch is updated per your suggestion. Is this one OK for current stage and 4.8/4.9?
>

This is OK for trunk. I suggest you let it gestate there for a few days 
before doing back-ports.

R.

> BR,
> Terry
>
> gcc/testsuite/ChangeLog:
>
> 2015-02-25  Terry Guo  <terry.guo@arm.com>
>
>      * gcc.target/arm/no-volatile-in-it.c: New test.
>
>
> gcc/ChangeLog:
>
> 2015-02-25  Terry Guo  <terry.guo@arm.com>
>
>      * config/arm/arm-cores.def (cortex-m7): Add flag FL_NO_VOLATILE_CE.
>      * config/arm/arm-protos.h (FL_NO_VOLATILE_CE): New flag.
>      (arm_arch_no_volatile_ce): Declare new global variable.
>      * config/arm/arm.c (arm_arch_no_volatile_ce): Define new global variable.
>      (arm_option_override): Assign value to arm_arch_no_volatile_ce.
>      * config/arm/arm.h (arm_arch_no_volatile_ce): Declare it.
>      (TARGET_NO_VOLATILE_CE): New macro.
>      * config/arm/arm.md (arm_comparison_operator): Disabled if not allow
>      volatile memory access in IT block
>