public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, GCC/ARM, gcc-6-branch] Fix PR80082: LDRD erronously used for 64bit load on ARMv7-R
@ 2017-03-27 11:15 Thomas Preudhomme
  2017-03-30 10:55 ` Thomas Preudhomme
  2017-04-06 13:05 ` [PATCH, GCC/ARM, gcc-6-branch] " Ramana Radhakrishnan
  0 siblings, 2 replies; 7+ messages in thread
From: Thomas Preudhomme @ 2017-03-27 11:15 UTC (permalink / raw)
  To: Kyrill Tkachov, Ramana Radhakrishnan, Richard Earnshaw, gcc-patches

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

Hi,

Currently GCC is happy to use LDRD to perform a 64bit load on ARMv7-R,
as shown by the testcase on this patch. However, LDRD is only atomic
when LPAE extensions is available, which they are not for ARMv7-R. This
commit solve the issue by introducing a new feature bit to distinguish
LPAE extensions instead of deducing it from div instruction
availability.

ChangeLog entries are as follow:

*** gcc/ChangeLog ***

2017-03-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>

	PR target/80082
	* config/arm/arm-protos.h (FL_LPAE): Define macro.
	(FL_FOR_ARCH7VE): Add FL_LPAE.
	(arm_arch_lpae): Declare extern.
	* config/arm/arm.c (arm_arch_lpae): Declare.
	(arm_option_override): Define arm_arch_lpae.
	* config/arm/arm.h (TARGET_HAVE_LPAE): Redefine in term of
	arm_arch_lpae.

*** gcc/testsuite/ChangeLog ***

2017-03-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>

	PR target/80082
	* gcc.target/arm/atomic_loaddi_10.c: New testcase.
	* gcc.target/arm/atomic_loaddi_11.c: Likewise.


Testing: bootstrapped for -march=armv7ve and testsuite shows no regression.

Is this ok for gcc-6-branch?

Best regards,

Thomas

[-- Attachment #2: atomic_loaddi_11_cortexr5_gcc6.patch --]
[-- Type: text/x-patch, Size: 4599 bytes --]

diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index 0083673b161a49e19388c72d3a413aeb481dbfa3..dea00e42551c8295f7e83a72ddb81ae8e9c8e02d 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -360,7 +360,7 @@ extern bool arm_is_constant_pool_ref (rtx);
 #define FL_STRONG     (1 << 8)	      /* StrongARM */
 #define FL_ARCH5E     (1 << 9)        /* DSP extensions to v5 */
 #define FL_XSCALE     (1 << 10)	      /* XScale */
-/* spare	      (1 << 11)	*/
+#define FL_LPAE       (1 << 11)       /* ARMv7-A LPAE.  */
 #define FL_ARCH6      (1 << 12)       /* Architecture rel 6.  Adds
 					 media instructions.  */
 #define FL_VFPV2      (1 << 13)       /* Vector Floating Point V2.  */
@@ -412,7 +412,7 @@ extern bool arm_is_constant_pool_ref (rtx);
 #define FL_FOR_ARCH6M	(FL_FOR_ARCH6 & ~FL_NOTM)
 #define FL_FOR_ARCH7	((FL_FOR_ARCH6T2 & ~FL_NOTM) | FL_ARCH7)
 #define FL_FOR_ARCH7A	(FL_FOR_ARCH7 | FL_NOTM | FL_ARCH6K)
-#define FL_FOR_ARCH7VE	(FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV)
+#define FL_FOR_ARCH7VE	(FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV | FL_LPAE)
 #define FL_FOR_ARCH7R	(FL_FOR_ARCH7A | FL_THUMB_DIV)
 #define FL_FOR_ARCH7M	(FL_FOR_ARCH7 | FL_THUMB_DIV)
 #define FL_FOR_ARCH7EM  (FL_FOR_ARCH7M | FL_ARCH7EM)
@@ -608,6 +608,9 @@ extern int arm_arch_thumb2;
 extern int arm_arch_arm_hwdiv;
 extern int arm_arch_thumb_hwdiv;
 
+/* Nonzero if this chip supports the Large Physical Address Extension.  */
+extern int arm_arch_lpae;
+
 /* Nonzero if chip disallows volatile memory access in IT block.  */
 extern int arm_arch_no_volatile_ce;
 
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index ad123dde991a3e4c4b9563ee6ebb84981767988f..e93ff7f7d8583b653570cbb8605df5a10bfcc6f4 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -254,8 +254,7 @@ extern void (*arm_lang_output_object_attributes_hook)(void);
 #define TARGET_HAVE_LDREX        ((arm_arch6 && TARGET_ARM) || arm_arch7)
 
 /* Nonzero if this chip supports LPAE.  */
-#define TARGET_HAVE_LPAE						\
-  (arm_arch7 && ARM_FSET_HAS_CPU1 (insn_flags, FL_FOR_ARCH7VE))
+#define TARGET_HAVE_LPAE	(arm_arch_lpae)
 
 /* Nonzero if this chip supports ldrex{bh} and strex{bh}.  */
 #define TARGET_HAVE_LDREXBH ((arm_arch6k && TARGET_ARM) || arm_arch7)
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index c3c89b866355708d91dd2a3dab1e4b33f2215ff8..44bfb53a288f57fbc43a0bd146e193b768d939d2 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -859,6 +859,9 @@ int arm_arch_thumb2;
 int arm_arch_arm_hwdiv;
 int arm_arch_thumb_hwdiv;
 
+/* Nonzero if this chip supports the Large Physical Address Extension.  */
+int arm_arch_lpae;
+
 /* Nonzero if chip disallows volatile memory access in IT block.  */
 int arm_arch_no_volatile_ce;
 
@@ -3181,6 +3184,7 @@ arm_option_override (void)
   arm_arch_iwmmxt2 = ARM_FSET_HAS_CPU1 (insn_flags, FL_IWMMXT2);
   arm_arch_thumb_hwdiv = ARM_FSET_HAS_CPU1 (insn_flags, FL_THUMB_DIV);
   arm_arch_arm_hwdiv = ARM_FSET_HAS_CPU1 (insn_flags, FL_ARM_DIV);
+  arm_arch_lpae = ARM_FSET_HAS_CPU1 (insn_flags, FL_LPAE);
   arm_arch_no_volatile_ce = ARM_FSET_HAS_CPU1 (insn_flags, FL_NO_VOLATILE_CE);
   arm_tune_cortex_a9 = (arm_tune == cortexa9) != 0;
   arm_arch_crc = ARM_FSET_HAS_CPU1 (insn_flags, FL_CRC32);
diff --git a/gcc/testsuite/gcc.target/arm/atomic_loaddi_10.c b/gcc/testsuite/gcc.target/arm/atomic_loaddi_10.c
new file mode 100644
index 0000000000000000000000000000000000000000..ecc3d06d0c9f5966daa3ce7e2d52e09d14e0cbc8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/atomic_loaddi_10.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_arch_v7ve_ok } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_arch_v7ve } */
+
+#include <stdatomic.h>
+
+atomic_llong x = 0;
+
+atomic_llong get_x()
+{
+  return atomic_load(&x);
+}
+
+/* { dg-final { scan-assembler "ldrd" } } */
diff --git a/gcc/testsuite/gcc.target/arm/atomic_loaddi_11.c b/gcc/testsuite/gcc.target/arm/atomic_loaddi_11.c
new file mode 100644
index 0000000000000000000000000000000000000000..85c64ae68b1b1ee68466809f7f83d07ceabec575
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/atomic_loaddi_11.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_arch_v7r_ok } */
+/* { dg-skip-if "do not override -mcpu" { *-*-* } { "-mcpu=*" "-march=*" } { "-mcpu=cortex-r5" } } */
+/* { dg-options "-O2 -mcpu=cortex-r5" } */
+
+#include <stdatomic.h>
+
+atomic_llong x = 0;
+
+atomic_llong get_x()
+{
+  return atomic_load(&x);
+}
+
+/* { dg-final { scan-assembler-not "ldrd" } } */

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH, GCC/ARM, gcc-6-branch] Fix PR80082: LDRD erronously used for 64bit load on ARMv7-R
  2017-03-27 11:15 [PATCH, GCC/ARM, gcc-6-branch] Fix PR80082: LDRD erronously used for 64bit load on ARMv7-R Thomas Preudhomme
@ 2017-03-30 10:55 ` Thomas Preudhomme
  2017-04-04 17:01   ` [PATCH, GCC/ARM, gcc-6-branch, ping2] " Thomas Preudhomme
  2017-04-06 13:05 ` [PATCH, GCC/ARM, gcc-6-branch] " Ramana Radhakrishnan
  1 sibling, 1 reply; 7+ messages in thread
From: Thomas Preudhomme @ 2017-03-30 10:55 UTC (permalink / raw)
  To: Kyrill Tkachov, Ramana Radhakrishnan, Richard Earnshaw, gcc-patches

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

Ping?

Best regards,

Thomas

On 27/03/17 12:15, Thomas Preudhomme wrote:
> Hi,
>
> Currently GCC is happy to use LDRD to perform a 64bit load on ARMv7-R,
> as shown by the testcase on this patch. However, LDRD is only atomic
> when LPAE extensions is available, which they are not for ARMv7-R. This
> commit solve the issue by introducing a new feature bit to distinguish
> LPAE extensions instead of deducing it from div instruction
> availability.
>
> ChangeLog entries are as follow:
>
> *** gcc/ChangeLog ***
>
> 2017-03-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>     PR target/80082
>     * config/arm/arm-protos.h (FL_LPAE): Define macro.
>     (FL_FOR_ARCH7VE): Add FL_LPAE.
>     (arm_arch_lpae): Declare extern.
>     * config/arm/arm.c (arm_arch_lpae): Declare.
>     (arm_option_override): Define arm_arch_lpae.
>     * config/arm/arm.h (TARGET_HAVE_LPAE): Redefine in term of
>     arm_arch_lpae.
>
> *** gcc/testsuite/ChangeLog ***
>
> 2017-03-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>     PR target/80082
>     * gcc.target/arm/atomic_loaddi_10.c: New testcase.
>     * gcc.target/arm/atomic_loaddi_11.c: Likewise.
>
>
> Testing: bootstrapped for -march=armv7ve and testsuite shows no regression.
>
> Is this ok for gcc-6-branch?
>
> Best regards,
>
> Thomas

[-- Attachment #2: atomic_loaddi_11_cortexr5_gcc6.patch --]
[-- Type: text/x-patch, Size: 4599 bytes --]

diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index 0083673b161a49e19388c72d3a413aeb481dbfa3..dea00e42551c8295f7e83a72ddb81ae8e9c8e02d 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -360,7 +360,7 @@ extern bool arm_is_constant_pool_ref (rtx);
 #define FL_STRONG     (1 << 8)	      /* StrongARM */
 #define FL_ARCH5E     (1 << 9)        /* DSP extensions to v5 */
 #define FL_XSCALE     (1 << 10)	      /* XScale */
-/* spare	      (1 << 11)	*/
+#define FL_LPAE       (1 << 11)       /* ARMv7-A LPAE.  */
 #define FL_ARCH6      (1 << 12)       /* Architecture rel 6.  Adds
 					 media instructions.  */
 #define FL_VFPV2      (1 << 13)       /* Vector Floating Point V2.  */
@@ -412,7 +412,7 @@ extern bool arm_is_constant_pool_ref (rtx);
 #define FL_FOR_ARCH6M	(FL_FOR_ARCH6 & ~FL_NOTM)
 #define FL_FOR_ARCH7	((FL_FOR_ARCH6T2 & ~FL_NOTM) | FL_ARCH7)
 #define FL_FOR_ARCH7A	(FL_FOR_ARCH7 | FL_NOTM | FL_ARCH6K)
-#define FL_FOR_ARCH7VE	(FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV)
+#define FL_FOR_ARCH7VE	(FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV | FL_LPAE)
 #define FL_FOR_ARCH7R	(FL_FOR_ARCH7A | FL_THUMB_DIV)
 #define FL_FOR_ARCH7M	(FL_FOR_ARCH7 | FL_THUMB_DIV)
 #define FL_FOR_ARCH7EM  (FL_FOR_ARCH7M | FL_ARCH7EM)
@@ -608,6 +608,9 @@ extern int arm_arch_thumb2;
 extern int arm_arch_arm_hwdiv;
 extern int arm_arch_thumb_hwdiv;
 
+/* Nonzero if this chip supports the Large Physical Address Extension.  */
+extern int arm_arch_lpae;
+
 /* Nonzero if chip disallows volatile memory access in IT block.  */
 extern int arm_arch_no_volatile_ce;
 
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index ad123dde991a3e4c4b9563ee6ebb84981767988f..e93ff7f7d8583b653570cbb8605df5a10bfcc6f4 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -254,8 +254,7 @@ extern void (*arm_lang_output_object_attributes_hook)(void);
 #define TARGET_HAVE_LDREX        ((arm_arch6 && TARGET_ARM) || arm_arch7)
 
 /* Nonzero if this chip supports LPAE.  */
-#define TARGET_HAVE_LPAE						\
-  (arm_arch7 && ARM_FSET_HAS_CPU1 (insn_flags, FL_FOR_ARCH7VE))
+#define TARGET_HAVE_LPAE	(arm_arch_lpae)
 
 /* Nonzero if this chip supports ldrex{bh} and strex{bh}.  */
 #define TARGET_HAVE_LDREXBH ((arm_arch6k && TARGET_ARM) || arm_arch7)
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index c3c89b866355708d91dd2a3dab1e4b33f2215ff8..44bfb53a288f57fbc43a0bd146e193b768d939d2 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -859,6 +859,9 @@ int arm_arch_thumb2;
 int arm_arch_arm_hwdiv;
 int arm_arch_thumb_hwdiv;
 
+/* Nonzero if this chip supports the Large Physical Address Extension.  */
+int arm_arch_lpae;
+
 /* Nonzero if chip disallows volatile memory access in IT block.  */
 int arm_arch_no_volatile_ce;
 
@@ -3181,6 +3184,7 @@ arm_option_override (void)
   arm_arch_iwmmxt2 = ARM_FSET_HAS_CPU1 (insn_flags, FL_IWMMXT2);
   arm_arch_thumb_hwdiv = ARM_FSET_HAS_CPU1 (insn_flags, FL_THUMB_DIV);
   arm_arch_arm_hwdiv = ARM_FSET_HAS_CPU1 (insn_flags, FL_ARM_DIV);
+  arm_arch_lpae = ARM_FSET_HAS_CPU1 (insn_flags, FL_LPAE);
   arm_arch_no_volatile_ce = ARM_FSET_HAS_CPU1 (insn_flags, FL_NO_VOLATILE_CE);
   arm_tune_cortex_a9 = (arm_tune == cortexa9) != 0;
   arm_arch_crc = ARM_FSET_HAS_CPU1 (insn_flags, FL_CRC32);
diff --git a/gcc/testsuite/gcc.target/arm/atomic_loaddi_10.c b/gcc/testsuite/gcc.target/arm/atomic_loaddi_10.c
new file mode 100644
index 0000000000000000000000000000000000000000..ecc3d06d0c9f5966daa3ce7e2d52e09d14e0cbc8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/atomic_loaddi_10.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_arch_v7ve_ok } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_arch_v7ve } */
+
+#include <stdatomic.h>
+
+atomic_llong x = 0;
+
+atomic_llong get_x()
+{
+  return atomic_load(&x);
+}
+
+/* { dg-final { scan-assembler "ldrd" } } */
diff --git a/gcc/testsuite/gcc.target/arm/atomic_loaddi_11.c b/gcc/testsuite/gcc.target/arm/atomic_loaddi_11.c
new file mode 100644
index 0000000000000000000000000000000000000000..85c64ae68b1b1ee68466809f7f83d07ceabec575
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/atomic_loaddi_11.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_arch_v7r_ok } */
+/* { dg-skip-if "do not override -mcpu" { *-*-* } { "-mcpu=*" "-march=*" } { "-mcpu=cortex-r5" } } */
+/* { dg-options "-O2 -mcpu=cortex-r5" } */
+
+#include <stdatomic.h>
+
+atomic_llong x = 0;
+
+atomic_llong get_x()
+{
+  return atomic_load(&x);
+}
+
+/* { dg-final { scan-assembler-not "ldrd" } } */

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH, GCC/ARM, gcc-6-branch, ping2] Fix PR80082: LDRD erronously used for 64bit load on ARMv7-R
  2017-03-30 10:55 ` Thomas Preudhomme
@ 2017-04-04 17:01   ` Thomas Preudhomme
  0 siblings, 0 replies; 7+ messages in thread
From: Thomas Preudhomme @ 2017-04-04 17:01 UTC (permalink / raw)
  To: Kyrill Tkachov, Ramana Radhakrishnan, Richard Earnshaw, gcc-patches

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

Ping?

Best regards,

Thomas

On 30/03/17 11:46, Thomas Preudhomme wrote:
> Ping?
>
> Best regards,
>
> Thomas
>
> On 27/03/17 12:15, Thomas Preudhomme wrote:
>> Hi,
>>
>> Currently GCC is happy to use LDRD to perform a 64bit load on ARMv7-R,
>> as shown by the testcase on this patch. However, LDRD is only atomic
>> when LPAE extensions is available, which they are not for ARMv7-R. This
>> commit solve the issue by introducing a new feature bit to distinguish
>> LPAE extensions instead of deducing it from div instruction
>> availability.
>>
>> ChangeLog entries are as follow:
>>
>> *** gcc/ChangeLog ***
>>
>> 2017-03-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>     PR target/80082
>>     * config/arm/arm-protos.h (FL_LPAE): Define macro.
>>     (FL_FOR_ARCH7VE): Add FL_LPAE.
>>     (arm_arch_lpae): Declare extern.
>>     * config/arm/arm.c (arm_arch_lpae): Declare.
>>     (arm_option_override): Define arm_arch_lpae.
>>     * config/arm/arm.h (TARGET_HAVE_LPAE): Redefine in term of
>>     arm_arch_lpae.
>>
>> *** gcc/testsuite/ChangeLog ***
>>
>> 2017-03-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>     PR target/80082
>>     * gcc.target/arm/atomic_loaddi_10.c: New testcase.
>>     * gcc.target/arm/atomic_loaddi_11.c: Likewise.
>>
>>
>> Testing: bootstrapped for -march=armv7ve and testsuite shows no regression.
>>
>> Is this ok for gcc-6-branch?
>>
>> Best regards,
>>
>> Thomas

[-- Attachment #2: atomic_loaddi_11_cortexr5_gcc6.patch --]
[-- Type: text/x-patch, Size: 4599 bytes --]

diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index 0083673b161a49e19388c72d3a413aeb481dbfa3..dea00e42551c8295f7e83a72ddb81ae8e9c8e02d 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -360,7 +360,7 @@ extern bool arm_is_constant_pool_ref (rtx);
 #define FL_STRONG     (1 << 8)	      /* StrongARM */
 #define FL_ARCH5E     (1 << 9)        /* DSP extensions to v5 */
 #define FL_XSCALE     (1 << 10)	      /* XScale */
-/* spare	      (1 << 11)	*/
+#define FL_LPAE       (1 << 11)       /* ARMv7-A LPAE.  */
 #define FL_ARCH6      (1 << 12)       /* Architecture rel 6.  Adds
 					 media instructions.  */
 #define FL_VFPV2      (1 << 13)       /* Vector Floating Point V2.  */
@@ -412,7 +412,7 @@ extern bool arm_is_constant_pool_ref (rtx);
 #define FL_FOR_ARCH6M	(FL_FOR_ARCH6 & ~FL_NOTM)
 #define FL_FOR_ARCH7	((FL_FOR_ARCH6T2 & ~FL_NOTM) | FL_ARCH7)
 #define FL_FOR_ARCH7A	(FL_FOR_ARCH7 | FL_NOTM | FL_ARCH6K)
-#define FL_FOR_ARCH7VE	(FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV)
+#define FL_FOR_ARCH7VE	(FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV | FL_LPAE)
 #define FL_FOR_ARCH7R	(FL_FOR_ARCH7A | FL_THUMB_DIV)
 #define FL_FOR_ARCH7M	(FL_FOR_ARCH7 | FL_THUMB_DIV)
 #define FL_FOR_ARCH7EM  (FL_FOR_ARCH7M | FL_ARCH7EM)
@@ -608,6 +608,9 @@ extern int arm_arch_thumb2;
 extern int arm_arch_arm_hwdiv;
 extern int arm_arch_thumb_hwdiv;
 
+/* Nonzero if this chip supports the Large Physical Address Extension.  */
+extern int arm_arch_lpae;
+
 /* Nonzero if chip disallows volatile memory access in IT block.  */
 extern int arm_arch_no_volatile_ce;
 
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index ad123dde991a3e4c4b9563ee6ebb84981767988f..e93ff7f7d8583b653570cbb8605df5a10bfcc6f4 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -254,8 +254,7 @@ extern void (*arm_lang_output_object_attributes_hook)(void);
 #define TARGET_HAVE_LDREX        ((arm_arch6 && TARGET_ARM) || arm_arch7)
 
 /* Nonzero if this chip supports LPAE.  */
-#define TARGET_HAVE_LPAE						\
-  (arm_arch7 && ARM_FSET_HAS_CPU1 (insn_flags, FL_FOR_ARCH7VE))
+#define TARGET_HAVE_LPAE	(arm_arch_lpae)
 
 /* Nonzero if this chip supports ldrex{bh} and strex{bh}.  */
 #define TARGET_HAVE_LDREXBH ((arm_arch6k && TARGET_ARM) || arm_arch7)
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index c3c89b866355708d91dd2a3dab1e4b33f2215ff8..44bfb53a288f57fbc43a0bd146e193b768d939d2 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -859,6 +859,9 @@ int arm_arch_thumb2;
 int arm_arch_arm_hwdiv;
 int arm_arch_thumb_hwdiv;
 
+/* Nonzero if this chip supports the Large Physical Address Extension.  */
+int arm_arch_lpae;
+
 /* Nonzero if chip disallows volatile memory access in IT block.  */
 int arm_arch_no_volatile_ce;
 
@@ -3181,6 +3184,7 @@ arm_option_override (void)
   arm_arch_iwmmxt2 = ARM_FSET_HAS_CPU1 (insn_flags, FL_IWMMXT2);
   arm_arch_thumb_hwdiv = ARM_FSET_HAS_CPU1 (insn_flags, FL_THUMB_DIV);
   arm_arch_arm_hwdiv = ARM_FSET_HAS_CPU1 (insn_flags, FL_ARM_DIV);
+  arm_arch_lpae = ARM_FSET_HAS_CPU1 (insn_flags, FL_LPAE);
   arm_arch_no_volatile_ce = ARM_FSET_HAS_CPU1 (insn_flags, FL_NO_VOLATILE_CE);
   arm_tune_cortex_a9 = (arm_tune == cortexa9) != 0;
   arm_arch_crc = ARM_FSET_HAS_CPU1 (insn_flags, FL_CRC32);
diff --git a/gcc/testsuite/gcc.target/arm/atomic_loaddi_10.c b/gcc/testsuite/gcc.target/arm/atomic_loaddi_10.c
new file mode 100644
index 0000000000000000000000000000000000000000..ecc3d06d0c9f5966daa3ce7e2d52e09d14e0cbc8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/atomic_loaddi_10.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_arch_v7ve_ok } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_arch_v7ve } */
+
+#include <stdatomic.h>
+
+atomic_llong x = 0;
+
+atomic_llong get_x()
+{
+  return atomic_load(&x);
+}
+
+/* { dg-final { scan-assembler "ldrd" } } */
diff --git a/gcc/testsuite/gcc.target/arm/atomic_loaddi_11.c b/gcc/testsuite/gcc.target/arm/atomic_loaddi_11.c
new file mode 100644
index 0000000000000000000000000000000000000000..85c64ae68b1b1ee68466809f7f83d07ceabec575
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/atomic_loaddi_11.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_arch_v7r_ok } */
+/* { dg-skip-if "do not override -mcpu" { *-*-* } { "-mcpu=*" "-march=*" } { "-mcpu=cortex-r5" } } */
+/* { dg-options "-O2 -mcpu=cortex-r5" } */
+
+#include <stdatomic.h>
+
+atomic_llong x = 0;
+
+atomic_llong get_x()
+{
+  return atomic_load(&x);
+}
+
+/* { dg-final { scan-assembler-not "ldrd" } } */

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH, GCC/ARM, gcc-6-branch] Fix PR80082: LDRD erronously used for 64bit load on ARMv7-R
  2017-03-27 11:15 [PATCH, GCC/ARM, gcc-6-branch] Fix PR80082: LDRD erronously used for 64bit load on ARMv7-R Thomas Preudhomme
  2017-03-30 10:55 ` Thomas Preudhomme
@ 2017-04-06 13:05 ` Ramana Radhakrishnan
  2017-04-06 16:38   ` Thomas Preudhomme
  2017-04-10 12:12   ` [PATCH, GCC/ARM, gcc-5-branch] " Thomas Preudhomme
  1 sibling, 2 replies; 7+ messages in thread
From: Ramana Radhakrishnan @ 2017-04-06 13:05 UTC (permalink / raw)
  To: Thomas Preudhomme
  Cc: Kyrill Tkachov, Ramana Radhakrishnan, Richard Earnshaw, gcc-patches

On Mon, Mar 27, 2017 at 12:15 PM, Thomas Preudhomme
<thomas.preudhomme@foss.arm.com> wrote:
> Hi,
>
> Currently GCC is happy to use LDRD to perform a 64bit load on ARMv7-R,
> as shown by the testcase on this patch. However, LDRD is only atomic
> when LPAE extensions is available, which they are not for ARMv7-R. This
> commit solve the issue by introducing a new feature bit to distinguish
> LPAE extensions instead of deducing it from div instruction
> availability.


Ok but with the testsuite fix that I just approved,  please also fix
in gcc-5 branch.

Thanks,
Ramana

>
> ChangeLog entries are as follow:
>
> *** gcc/ChangeLog ***
>
> 2017-03-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>         PR target/80082
>         * config/arm/arm-protos.h (FL_LPAE): Define macro.
>         (FL_FOR_ARCH7VE): Add FL_LPAE.
>         (arm_arch_lpae): Declare extern.
>         * config/arm/arm.c (arm_arch_lpae): Declare.
>         (arm_option_override): Define arm_arch_lpae.
>         * config/arm/arm.h (TARGET_HAVE_LPAE): Redefine in term of
>         arm_arch_lpae.
>
> *** gcc/testsuite/ChangeLog ***
>
> 2017-03-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>         PR target/80082
>         * gcc.target/arm/atomic_loaddi_10.c: New testcase.
>         * gcc.target/arm/atomic_loaddi_11.c: Likewise.
>
>
> Testing: bootstrapped for -march=armv7ve and testsuite shows no regression.
>
> Is this ok for gcc-6-branch?
>
> Best regards,
>
> Thomas

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH, GCC/ARM, gcc-6-branch] Fix PR80082: LDRD erronously used for 64bit load on ARMv7-R
  2017-04-06 13:05 ` [PATCH, GCC/ARM, gcc-6-branch] " Ramana Radhakrishnan
@ 2017-04-06 16:38   ` Thomas Preudhomme
  2017-04-10 12:12   ` [PATCH, GCC/ARM, gcc-5-branch] " Thomas Preudhomme
  1 sibling, 0 replies; 7+ messages in thread
From: Thomas Preudhomme @ 2017-04-06 16:38 UTC (permalink / raw)
  To: Ramana Radhakrishnan
  Cc: Kyrill Tkachov, Ramana Radhakrishnan, Richard Earnshaw, gcc-patches



On 06/04/17 14:05, Ramana Radhakrishnan wrote:
> On Mon, Mar 27, 2017 at 12:15 PM, Thomas Preudhomme
> <thomas.preudhomme@foss.arm.com> wrote:
>> Hi,
>>
>> Currently GCC is happy to use LDRD to perform a 64bit load on ARMv7-R,
>> as shown by the testcase on this patch. However, LDRD is only atomic
>> when LPAE extensions is available, which they are not for ARMv7-R. This
>> commit solve the issue by introducing a new feature bit to distinguish
>> LPAE extensions instead of deducing it from div instruction
>> availability.
>
>
> Ok but with the testsuite fix that I just approved,  please also fix
> in gcc-5 branch.

The backport already contains it. I haven't asked for gcc-5 branch yet because 
testing is still ongoing. Will send a separate mail once testing is done.

Best regards,

Thomas

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH, GCC/ARM, gcc-5-branch] Fix PR80082: LDRD erronously used for 64bit load on ARMv7-R
  2017-04-06 13:05 ` [PATCH, GCC/ARM, gcc-6-branch] " Ramana Radhakrishnan
  2017-04-06 16:38   ` Thomas Preudhomme
@ 2017-04-10 12:12   ` Thomas Preudhomme
  2017-04-11 14:47     ` Ramana Radhakrishnan
  1 sibling, 1 reply; 7+ messages in thread
From: Thomas Preudhomme @ 2017-04-10 12:12 UTC (permalink / raw)
  To: Ramana Radhakrishnan
  Cc: Kyrill Tkachov, Ramana Radhakrishnan, Richard Earnshaw, gcc-patches

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

Hi,

Currently GCC is happy to use LDRD to perform a 64bit load on ARMv7-R,
as shown by the testcase on this patch. However, LDRD is only atomic
when LPAE extensions is available, which they are not for ARMv7-R. This
commit solve the issue by introducing a new feature bit to distinguish
LPAE extensions instead of deducing it from div instruction
availability.

ChangeLog entries are as follow:

*** gcc/ChangeLog ***

2017-03-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>

	PR target/80082
	* config/arm/arm-protos.h (FL_LPAE): Define macro.
	(FL_FOR_ARCH7VE): Add FL_LPAE.
	(arm_arch_lpae): Declare extern.
	* config/arm/arm.c (arm_arch_lpae): Declare.
	(arm_option_override): Define arm_arch_lpae.
	* config/arm/arm.h (TARGET_HAVE_LPAE): Redefine in term of
	arm_arch_lpae.

*** gcc/testsuite/ChangeLog ***

2017-03-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>

	PR target/80082
	* gcc.target/arm/atomic_loaddi_10.c: New testcase.
	* gcc.target/arm/atomic_loaddi_11.c: Likewise.

Is this ok for gcc-5-branch?

Best regards,

Thomas

On 06/04/17 14:05, Ramana Radhakrishnan wrote:
> On Mon, Mar 27, 2017 at 12:15 PM, Thomas Preudhomme
> <thomas.preudhomme@foss.arm.com> wrote:
>> Hi,
>>
>> Currently GCC is happy to use LDRD to perform a 64bit load on ARMv7-R,
>> as shown by the testcase on this patch. However, LDRD is only atomic
>> when LPAE extensions is available, which they are not for ARMv7-R. This
>> commit solve the issue by introducing a new feature bit to distinguish
>> LPAE extensions instead of deducing it from div instruction
>> availability.
>
>
> Ok but with the testsuite fix that I just approved,  please also fix
> in gcc-5 branch.
>
> Thanks,
> Ramana
>
>>
>> ChangeLog entries are as follow:
>>
>> *** gcc/ChangeLog ***
>>
>> 2017-03-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>         PR target/80082
>>         * config/arm/arm-protos.h (FL_LPAE): Define macro.
>>         (FL_FOR_ARCH7VE): Add FL_LPAE.
>>         (arm_arch_lpae): Declare extern.
>>         * config/arm/arm.c (arm_arch_lpae): Declare.
>>         (arm_option_override): Define arm_arch_lpae.
>>         * config/arm/arm.h (TARGET_HAVE_LPAE): Redefine in term of
>>         arm_arch_lpae.
>>
>> *** gcc/testsuite/ChangeLog ***
>>
>> 2017-03-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>         PR target/80082
>>         * gcc.target/arm/atomic_loaddi_10.c: New testcase.
>>         * gcc.target/arm/atomic_loaddi_11.c: Likewise.
>>
>>
>> Testing: bootstrapped for -march=armv7ve and testsuite shows no regression.
>>
>> Is this ok for gcc-6-branch?
>>
>> Best regards,
>>
>> Thomas

[-- Attachment #2: atomic_loaddi_11_cortexr5_gcc5.patch --]
[-- Type: text/x-patch, Size: 4523 bytes --]

diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index ebaf746227723f68c4c1cda89ba35c598a3ae4ba..21df391789d3d9f7b1d28e2bb2f99458843d0c3e 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -343,7 +343,7 @@ extern bool arm_is_constant_pool_ref (rtx);
 #define FL_STRONG     (1 << 8)	      /* StrongARM */
 #define FL_ARCH5E     (1 << 9)        /* DSP extensions to v5 */
 #define FL_XSCALE     (1 << 10)	      /* XScale */
-/* spare	      (1 << 11)	*/
+#define FL_LPAE       (1 << 11)       /* ARMv7-A LPAE.  */
 #define FL_ARCH6      (1 << 12)       /* Architecture rel 6.  Adds
 					 media instructions.  */
 #define FL_VFPV2      (1 << 13)       /* Vector Floating Point V2.  */
@@ -392,7 +392,7 @@ extern bool arm_is_constant_pool_ref (rtx);
 #define FL_FOR_ARCH6M	(FL_FOR_ARCH6 & ~FL_NOTM)
 #define FL_FOR_ARCH7	((FL_FOR_ARCH6T2 & ~FL_NOTM) | FL_ARCH7)
 #define FL_FOR_ARCH7A	(FL_FOR_ARCH7 | FL_NOTM | FL_ARCH6K)
-#define FL_FOR_ARCH7VE	(FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV)
+#define FL_FOR_ARCH7VE	(FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV | FL_LPAE)
 #define FL_FOR_ARCH7R	(FL_FOR_ARCH7A | FL_THUMB_DIV)
 #define FL_FOR_ARCH7M	(FL_FOR_ARCH7 | FL_THUMB_DIV)
 #define FL_FOR_ARCH7EM  (FL_FOR_ARCH7M | FL_ARCH7EM)
@@ -487,6 +487,9 @@ extern int arm_arch_thumb2;
 extern int arm_arch_arm_hwdiv;
 extern int arm_arch_thumb_hwdiv;
 
+/* Nonzero if this chip supports the Large Physical Address Extension.  */
+extern int arm_arch_lpae;
+
 /* Nonzero if chip disallows volatile memory access in IT block.  */
 extern int arm_arch_no_volatile_ce;
 
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 5561e433b2929df311aec3263033c14b366836d3..4baf3e454b293c765c98948d86b23024ef84982d 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -370,8 +370,7 @@ extern void (*arm_lang_output_object_attributes_hook)(void);
 #define TARGET_HAVE_LDREX	((arm_arch6 && TARGET_ARM) || arm_arch7)
 
 /* Nonzero if this chip supports LPAE.  */
-#define TARGET_HAVE_LPAE						\
-  (arm_arch7 && ((insn_flags & FL_FOR_ARCH7VE) == FL_FOR_ARCH7VE))
+#define TARGET_HAVE_LPAE	(arm_arch_lpae)
 
 /* Nonzero if this chip supports ldrex{bh} and strex{bh}.  */
 #define TARGET_HAVE_LDREXBH	((arm_arch6k && TARGET_ARM) || arm_arch7)
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 6f9518995a387c84cd53deaa5ae6bac465d81ee8..c8aab8e09f83efd084a9d21e0d2fd04c04120877 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 this chip supports the Large Physical Address Extension.  */
+int arm_arch_lpae;
+
 /* Nonzero if chip disallows volatile memory access in IT block.  */
 int arm_arch_no_volatile_ce;
 
@@ -2862,6 +2865,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_lpae = (insn_flags & FL_LPAE) != 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;
diff --git a/gcc/testsuite/gcc.target/arm/atomic_loaddi_10.c b/gcc/testsuite/gcc.target/arm/atomic_loaddi_10.c
new file mode 100644
index 0000000000000000000000000000000000000000..ecc3d06d0c9f5966daa3ce7e2d52e09d14e0cbc8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/atomic_loaddi_10.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_arch_v7ve_ok } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_arch_v7ve } */
+
+#include <stdatomic.h>
+
+atomic_llong x = 0;
+
+atomic_llong get_x()
+{
+  return atomic_load(&x);
+}
+
+/* { dg-final { scan-assembler "ldrd" } } */
diff --git a/gcc/testsuite/gcc.target/arm/atomic_loaddi_11.c b/gcc/testsuite/gcc.target/arm/atomic_loaddi_11.c
new file mode 100644
index 0000000000000000000000000000000000000000..85c64ae68b1b1ee68466809f7f83d07ceabec575
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/atomic_loaddi_11.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_arch_v7r_ok } */
+/* { dg-skip-if "do not override -mcpu" { *-*-* } { "-mcpu=*" "-march=*" } { "-mcpu=cortex-r5" } } */
+/* { dg-options "-O2 -mcpu=cortex-r5" } */
+
+#include <stdatomic.h>
+
+atomic_llong x = 0;
+
+atomic_llong get_x()
+{
+  return atomic_load(&x);
+}
+
+/* { dg-final { scan-assembler-not "ldrd" } } */

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH, GCC/ARM, gcc-5-branch] Fix PR80082: LDRD erronously used for 64bit load on ARMv7-R
  2017-04-10 12:12   ` [PATCH, GCC/ARM, gcc-5-branch] " Thomas Preudhomme
@ 2017-04-11 14:47     ` Ramana Radhakrishnan
  0 siblings, 0 replies; 7+ messages in thread
From: Ramana Radhakrishnan @ 2017-04-11 14:47 UTC (permalink / raw)
  To: Thomas Preudhomme, Ramana Radhakrishnan
  Cc: Kyrill Tkachov, Richard Earnshaw, gcc-patches

On 10/04/17 13:12, Thomas Preudhomme wrote:
> Hi,
>
> Currently GCC is happy to use LDRD to perform a 64bit load on ARMv7-R,
> as shown by the testcase on this patch. However, LDRD is only atomic
> when LPAE extensions is available, which they are not for ARMv7-R. This
> commit solve the issue by introducing a new feature bit to distinguish
> LPAE extensions instead of deducing it from div instruction
> availability.
>
> ChangeLog entries are as follow:
>
> *** gcc/ChangeLog ***
>
> 2017-03-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>     PR target/80082
>     * config/arm/arm-protos.h (FL_LPAE): Define macro.
>     (FL_FOR_ARCH7VE): Add FL_LPAE.
>     (arm_arch_lpae): Declare extern.
>     * config/arm/arm.c (arm_arch_lpae): Declare.
>     (arm_option_override): Define arm_arch_lpae.
>     * config/arm/arm.h (TARGET_HAVE_LPAE): Redefine in term of
>     arm_arch_lpae.
>
> *** gcc/testsuite/ChangeLog ***
>
> 2017-03-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>     PR target/80082
>     * gcc.target/arm/atomic_loaddi_10.c: New testcase.
>     * gcc.target/arm/atomic_loaddi_11.c: Likewise.
>
> Is this ok for gcc-5-branch?

OK.

Ramana

>
> Best regards,
>
> Thomas
>
> On 06/04/17 14:05, Ramana Radhakrishnan wrote:
>> On Mon, Mar 27, 2017 at 12:15 PM, Thomas Preudhomme
>> <thomas.preudhomme@foss.arm.com> wrote:
>>> Hi,
>>>
>>> Currently GCC is happy to use LDRD to perform a 64bit load on ARMv7-R,
>>> as shown by the testcase on this patch. However, LDRD is only atomic
>>> when LPAE extensions is available, which they are not for ARMv7-R. This
>>> commit solve the issue by introducing a new feature bit to distinguish
>>> LPAE extensions instead of deducing it from div instruction
>>> availability.
>>
>>
>> Ok but with the testsuite fix that I just approved,  please also fix
>> in gcc-5 branch.
>>
>> Thanks,
>> Ramana
>>
>>>
>>> ChangeLog entries are as follow:
>>>
>>> *** gcc/ChangeLog ***
>>>
>>> 2017-03-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>>
>>>         PR target/80082
>>>         * config/arm/arm-protos.h (FL_LPAE): Define macro.
>>>         (FL_FOR_ARCH7VE): Add FL_LPAE.
>>>         (arm_arch_lpae): Declare extern.
>>>         * config/arm/arm.c (arm_arch_lpae): Declare.
>>>         (arm_option_override): Define arm_arch_lpae.
>>>         * config/arm/arm.h (TARGET_HAVE_LPAE): Redefine in term of
>>>         arm_arch_lpae.
>>>
>>> *** gcc/testsuite/ChangeLog ***
>>>
>>> 2017-03-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>>
>>>         PR target/80082
>>>         * gcc.target/arm/atomic_loaddi_10.c: New testcase.
>>>         * gcc.target/arm/atomic_loaddi_11.c: Likewise.
>>>
>>>
>>> Testing: bootstrapped for -march=armv7ve and testsuite shows no
>>> regression.
>>>
>>> Is this ok for gcc-6-branch?
>>>
>>> Best regards,
>>>
>>> Thomas

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-04-11 14:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-27 11:15 [PATCH, GCC/ARM, gcc-6-branch] Fix PR80082: LDRD erronously used for 64bit load on ARMv7-R Thomas Preudhomme
2017-03-30 10:55 ` Thomas Preudhomme
2017-04-04 17:01   ` [PATCH, GCC/ARM, gcc-6-branch, ping2] " Thomas Preudhomme
2017-04-06 13:05 ` [PATCH, GCC/ARM, gcc-6-branch] " Ramana Radhakrishnan
2017-04-06 16:38   ` Thomas Preudhomme
2017-04-10 12:12   ` [PATCH, GCC/ARM, gcc-5-branch] " Thomas Preudhomme
2017-04-11 14:47     ` Ramana Radhakrishnan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).