| Message ID | 56FD4FC1.70408@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
On 31 March 2016 at 18:26, Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> wrote: > > 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. > Thanks, I confirm that this fixed the build and that the new test passes in the 4.9 branch. For the 5 branch, the validation is still running, I'll warn you if there is any problem. Thanks Christophe > 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; +}