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.9 Backport] PR target/69875 Fix atomic_loaddi expansion


On 30/03/16 09:34, Kyrill Tkachov wrote:


On 29/03/16 19:46, Christophe Lyon wrote:

On 16 March 2016 at 16:54, Ramana Radhakrishnan
<ramana.gcc@googlemail.com> wrote:

On Wed, Feb 24, 2016 at 11:23 AM, Kyrill Tkachov
<kyrylo.tkachov@foss.arm.com> wrote:

Hi all,

This is the GCC 4.9 backport of
https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01338.html.
The differences are that TARGET_HAVE_LPAE has to be defined in arm.h in a
different way because
the ARM_FSET_HAS_CPU1 mechanism doesn't exist on this branch. Also, due to
the location of insn_flags
and the various FL_* (on the 4.9 branch they're defined locally in arm.c
rather than in arm-protos.h)
I chose to define TARGET_HAVE_LPAE in terms of hardware divide instruction
availability. This should be
an equivalent definition.

Also, the scan-assembler tests that check for the DMB instruction are
updated to check for
"dmb sy" rather than "dmb ish", because the memory barrier instruction
changed on trunk for GCC 6.

Bootstrapped and tested on the GCC 4.9 branch on arm-none-linux-gnueabihf.


Ok for the branch after the trunk patch has had a few days to bake?


OK.


Hi Kyrylo,

Since you backported this to branches 4.9 and 5, I've noticed cross-GCC build
failures:
--target arm-none-linux-gnueabihf
--with-mode=arm
--with-cpu=cortex-a57
--with-fpu=crypto-neon-fp-armv8

The build succeeds --with-mode=thumb.

The error message I'm seeing is:
/tmp/6190285_22.tmpdir/ccuX17sh.s: Assembler messages:
/tmp/6190285_22.tmpdir/ccuX17sh.s:34: Error: bad instruction `ldrdeq r0,r1,[r0]'
make[4]: *** [load_8_.lo] Error 1

while building libatomic


Darn, I had re-tested before committing with --with-mode=thumb :(
The problem here is that GCC 5 and 4.9 don't use unified syntax
for arm state (it was switched on for GCC 6), so the output template
in the new arm_atomic_loaddi2_ldrd pattern should be "ldr%(d%)" instead
of "ldrd%?".

I'll prepare a patch.
Thanks for catching this,
Kyrill



And here it is.
I've reproduced the build failure on 4.9 and 5 and confirmed that this patch fixes
them and that a build with --with-mode=thumb is unaffected.

I'm committing this as obvious in order to fix the broken build in the affected configurations.

Sorry for the trouble.

Kyrill

2016-03-31  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

    * config/arm/sync.md (arm_atomic_loaddi2_ldrd): Fix output template
    for non-unified syntax.

2016-03-31  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

    * gcc.target/arm/atomic_loaddi_relaxed_cond.c: New test.




Christophe



Ramana

Thanks,
Kyrill

2016-02-24  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR target/69875
     * config/arm/arm.h (TARGET_HAVE_LPAE): Define.
     * config/arm/unspecs.md (VUNSPEC_LDRD_ATOMIC): New value.
     * config/arm/sync.md (arm_atomic_loaddi2_ldrd): New pattern.
     (atomic_loaddi_1): Delete.
     (atomic_loaddi): Rewrite expander using the above changes.

2016-02-24  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR target/69875
     * gcc.target/arm/atomic_loaddi_acquire.x: New file.
     * gcc.target/arm/atomic_loaddi_relaxed.x: Likewise.
     * gcc.target/arm/atomic_loaddi_seq_cst.x: Likewise.
     * gcc.target/arm/atomic_loaddi_1.c: New test.
     * gcc.target/arm/atomic_loaddi_2.c: Likewise.
     * gcc.target/arm/atomic_loaddi_3.c: Likewise.
     * gcc.target/arm/atomic_loaddi_4.c: Likewise.
     * gcc.target/arm/atomic_loaddi_5.c: Likewise.
     * gcc.target/arm/atomic_loaddi_6.c: Likewise.
     * gcc.target/arm/atomic_loaddi_7.c: Likewise.
     * gcc.target/arm/atomic_loaddi_8.c: Likewise.
     * gcc.target/arm/atomic_loaddi_9.c: Likewise.





diff --git a/gcc/config/arm/sync.md b/gcc/config/arm/sync.md
index acafd0a6ec474466ff7e2c67ae16d9a0dbb9cf5c..17eab93d55c00fd8db3f612bed1caf0e29335f27 100644
--- a/gcc/config/arm/sync.md
+++ b/gcc/config/arm/sync.md
@@ -107,7 +107,7 @@ (define_insn "arm_atomic_loaddi2_ldrd"
 	  [(match_operand:DI 1 "arm_sync_memory_operand" "Q")]
 	    VUNSPEC_LDRD_ATOMIC))]
   "ARM_DOUBLEWORD_ALIGN && TARGET_HAVE_LPAE"
-  "ldrd%?\t%0, %H0, %C1"
+  "ldr%(d%)\t%0, %H0, %C1"
   [(set_attr "predicable" "yes")
    (set_attr "predicable_short_it" "no")])
 
diff --git a/gcc/testsuite/gcc.target/arm/atomic_loaddi_relaxed_cond.c b/gcc/testsuite/gcc.target/arm/atomic_loaddi_relaxed_cond.c
new file mode 100644
index 0000000000000000000000000000000000000000..d69775150813a01b7fcab64deac218a6b2c33c56
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/atomic_loaddi_relaxed_cond.c
@@ -0,0 +1,20 @@
+/* { dg-do assemble } */
+/* { dg-options "-std=c11 -O" } */
+/* { dg-require-effective-target arm_arch_v8a_ok } */
+/* { dg-add-options arm_arch_v8a } */
+
+/* Check that if we conditionalise the atomic load we put the condition
+   code in the right place to create valid assembly.  */
+
+#include <stdatomic.h>
+
+atomic_ullong foo;
+int glob;
+
+int
+main (int argc, char *argv[])
+{
+  if (argc > 2)
+    atomic_load_explicit (&foo, memory_order_relaxed);
+  return glob;
+}
Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]