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 7/7] Enable atomics for ARMv8-M Mainline


On Thursday 14 July 2016 17:23:46 Kyrill Tkachov wrote:
> Hi Thomas,
> 
> On 14/07/16 14:37, Thomas Preudhomme wrote:
> > Hi Kyrill,
> > 
> > On Thursday 19 May 2016 17:18:29 Kyrill Tkachov wrote:
> >> Hi Thomas,
> >> 
> >> On 17/05/16 11:15, Thomas Preudhomme wrote:
> >>> Ping?
> >>> 
> >>> *** gcc/ChangeLog ***
> >>> 
> >>> 2015-12-17  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> >>> 
> >>>           * config/arm/arm.h (TARGET_HAVE_LDACQ): Enable for ARMv8-M
> >>>           Mainline.
> >>> 
> >>> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
> >>> index
> >>> 347b5b0a5cc0bc1e3b5020c8124d968e76ce48a4..e154bd31b8084f9f45ad4409e7b38d
> >>> e6
> >>> 52538c51 100644
> >>> --- a/gcc/config/arm/arm.h
> >>> +++ b/gcc/config/arm/arm.h
> >>> @@ -266,7 +266,7 @@ extern void
> >>> (*arm_lang_output_object_attributes_hook)
> >>> (void);
> >>> 
> >>>    			     || arm_arch7) && arm_arch_notm)
> >>>    
> >>>    /* Nonzero if this chip supports load-acquire and store-release.  */
> >>> 
> >>> -#define TARGET_HAVE_LDACQ	(TARGET_ARM_ARCH >= 8 && arm_arch_notm)
> >>> +#define TARGET_HAVE_LDACQ	(TARGET_ARM_ARCH >= 8 && TARGET_32BIT)
> >> 
> >> So this change is correct because ARMv8-M Mainline uses Thumb2
> >> and is therefore TARGET_32BIT.
> >> 
> >> This is ok but I'd like to see a follow up patch to enable the tests
> >> that exercise acquire-release instructions in the arm.exp testsuite
> >> for ARMv8-M Mainline so that we can be sure they get proper testsuite
> >> coverage.
> > 
> > I've respinned the patch because of the changes to atomic_loaddi output
> > template in config/arm/sync.md. This patch now creates a new macro
> > TARGET_HAVE_LDACQEXD to guard LDACQEXD and STLEXD instructions that are
> > not
> > available in ARMv8-M Mainline. It took advantage of the respin to also add
> > the tests you were asking for.
> > 
> > ChangeLog entries are as follow:
> > 
> > *** gcc/ChangeLog ***
> > 
> > 2016-07-05  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> > 
> >          * config/arm/arm.h (TARGET_HAVE_LDACQ): Enable for ARMv8-M
> >          Mainline.
> >          (TARGET_HAVE_LDACQD): New macro.
> >          * config/arm/sync.md (atomic_loaddi): Use TARGET_HAVE_LDACQD
> >          rather
> >          than TARGET_HAVE_LDACQ.
> >          (arm_load_acquire_exclusivedi): Likewise.
> >          (arm_store_release_exclusivedi): Likewise.
> > 
> > *** gcc/testsuite/ChangeLog ***
> > 
> > 2016-07-05  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> > 
> >          * gcc.target/arm/atomic-comp-swap-release-acquire.c: Rename into
> >          ...
> >          * gcc.target/arm/atomic-comp-swap-release-acquire-1.c: This.
> >          * gcc.target/arm/atomic-op-acq_rel.c: Rename into ...
> >          * gcc.target/arm/atomic-op-acq_rel-1.c: This.
> >          * gcc.target/arm/atomic-op-acquire.c: Rename into ...
> >          * gcc.target/arm/atomic-op-acquire-1.c: This.
> >          * gcc.target/arm/atomic-op-char.c: Rename into ...
> >          * gcc.target/arm/atomic-op-char-1.c: This.
> >          * gcc.target/arm/atomic-op-consume.c: Rename into ...
> >          * gcc.target/arm/atomic-op-consume-1.c: This.
> >          * gcc.target/arm/atomic-op-int.c: Rename into ...
> >          * gcc.target/arm/atomic-op-int-1.c: This.
> >          * gcc.target/arm/atomic-op-relaxed.c: Rename into ...
> >          * gcc.target/arm/atomic-op-relaxed-1.c: This.
> >          * gcc.target/arm/atomic-op-release.c: Rename into ...
> >          * gcc.target/arm/atomic-op-release-1.c: This.
> >          * gcc.target/arm/atomic-op-seq_cst.c: Rename into ...
> >          * gcc.target/arm/atomic-op-seq_cst-1.c: This.
> >          * gcc.target/arm/atomic-op-short.c: Rename into ...
> >          * gcc.target/arm/atomic-op-short-1.c: This.
> >          * gcc.target/arm/atomic-comp-swap-release-acquire-2.c: New test.
> >          * gcc.target/arm/atomic-op-acq_rel-2.c: Likewise.
> >          * gcc.target/arm/atomic-op-acquire-2.c: Likewise.
> >          * gcc.target/arm/atomic-op-char-2.c: Likewise.
> >          * gcc.target/arm/atomic-op-consume-2.c: Likewise.
> >          * gcc.target/arm/atomic-op-int-2.c: Likewise.
> >          * gcc.target/arm/atomic-op-relaxed-2.c: Likewise.
> >          * gcc.target/arm/atomic-op-release-2.c: Likewise.
> >          * gcc.target/arm/atomic-op-seq_cst-2.c: Likewise.
> >          * gcc.target/arm/atomic-op-short-2.c: Likewise.
> > 
> > Testsuite shows no regression and atomic tests [1] are passing for ARMv8-M
> > Mainline.
> > 
> > [1] gcc.dg/atomic*, g++.dg/ext/atomic*, gcc.target/arm/atomic*,
> > gcc.target/arm/sync* and libstdc++-v3/testsuite/29_atomic/*
> 
> Thanks, this is ok if testing on arm-none-linux-gnueabihf is ok.
> In particular, please make sure that the tests still pass for an A-profile
> target.

Oh yes, I forgot to mention that I also tested the same tests for armv8-a 
without any code generation change for both ARM and Thumb. The whole testsuite 
shows no regression as well.

Best regards,

Thomas


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]