From: Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>
To: Thomas Preudhomme <thomas.preudhomme@foss.arm.com>
Cc: Richard Earnshaw <Richard.Earnshaw@arm.com>,
Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>,
gcc-patches@gcc.gnu.org
Subject: Re: [PATCH, ARM 7/7] Enable atomics for ARMv8-M Mainline
Date: Thu, 14 Jul 2016 16:24:00 -0000 [thread overview]
Message-ID: <5787BC92.9020803@foss.arm.com> (raw)
In-Reply-To: <7032225.jWGXdUoXx6@e108577-lin>
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..e154bd31b8084f9f45ad4409e7b38de6
>>> 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.
Kyrill
>
> Best regards,
>
> Thomas
next prev parent reply other threads:[~2016-07-14 16:24 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-17 9:39 [PATCH, ARM 7/6] " Thomas Preud'homme
2016-05-17 10:15 ` [PATCH, ARM 7/7, ping1] " Thomas Preudhomme
2016-05-19 16:18 ` Kyrill Tkachov
2016-05-19 16:48 ` Thomas Preudhomme
2016-07-14 13:38 ` [PATCH, ARM 7/7] " Thomas Preudhomme
2016-07-14 16:24 ` Kyrill Tkachov [this message]
2016-07-14 16:35 ` Thomas Preudhomme
2016-09-01 12:43 ` [arm-embedded] " Thomas Preudhomme
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5787BC92.9020803@foss.arm.com \
--to=kyrylo.tkachov@foss.arm.com \
--cc=Richard.Earnshaw@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=ramana.radhakrishnan@arm.com \
--cc=thomas.preudhomme@foss.arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).