diff mbox

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

Message ID 000901d050e7$c02322e0$406968a0$@arm.com
State New
Headers show

Commit Message

Terry Guo Feb. 25, 2015, 10:42 a.m. UTC 
> -----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

Comments

Richard Earnshaw Feb. 25, 2015, 11:04 a.m. UTC | #1 
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
>
diff mbox

Patch

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" } } */