diff mbox

[libstdc++/testsuite,ping] 29_atomics/atomic/65913.cc: require atomic-builtins rather than specific target

Message ID 2866975.ctxglMszaX@e108577-lin
State New
Headers show

Commit Message

Thomas Preudhomme June 2, 2016, 1:34 p.m. UTC
Ping?

On Thursday 26 May 2016 14:00:55 Thomas Preudhomme wrote:
> [Sorry for the large recipient list, I wasn't sure who of C++ and x86
> maintainers should approve this]
> 
> Hi,
> 
> 29_atomics/atomic/65913.cc test in libstdc++ is a runtime test that only
> rely on atomic and gnu++11 support. Therefore I propose to require
> atomic-builtins instead of an x86 (32 or 64 bits) target.
> 
> ChangeLog entry is as follows:
> 
> 2016-05-19  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * testsuite/29_atomics/atomic/65913.cc: Require atomic-builtins
> rather than specific target.
> 
> 
> Patch is in attachment.
> 
> 
> Is this ok for trunk?
> 
> Best regards,
> 
> Thomas

Comments

Thomas Preudhomme June 29, 2016, 4:35 p.m. UTC | #1
Ping?

Best regards,

Thomas

On Thursday 02 June 2016 14:34:03 Thomas Preudhomme wrote:
> Ping?
> 
> On Thursday 26 May 2016 14:00:55 Thomas Preudhomme wrote:
> > [Sorry for the large recipient list, I wasn't sure who of C++ and x86
> > maintainers should approve this]
> > 
> > Hi,
> > 
> > 29_atomics/atomic/65913.cc test in libstdc++ is a runtime test that only
> > rely on atomic and gnu++11 support. Therefore I propose to require
> > atomic-builtins instead of an x86 (32 or 64 bits) target.
> > 
> > ChangeLog entry is as follows:
> > 
> > 2016-05-19  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> > 
> >         * testsuite/29_atomics/atomic/65913.cc: Require atomic-builtins
> > 
> > rather than specific target.
> > 
> > 
> > Patch is in attachment.
> > 
> > 
> > Is this ok for trunk?
> > 
> > Best regards,
> > 
> > Thomas
Mike Stump June 29, 2016, 8:49 p.m. UTC | #2
Please include the libstdc++ list, they don't all read the other list.  Also, the patch or a link to the patch helps the reviewers find the patch, otherwise even finding the patch to review can be hard for some folks.

Seems reasonable to me, though, I'd normally punt to the atomic people.

> On Jun 29, 2016, at 9:35 AM, Thomas Preudhomme <thomas.preudhomme@foss.arm.com> wrote:
> Ping?
> 
> Best regards,
> 
> Thomas
> 
> On Thursday 02 June 2016 14:34:03 Thomas Preudhomme wrote:
>> Ping?
>> 
>> On Thursday 26 May 2016 14:00:55 Thomas Preudhomme wrote:
>>> [Sorry for the large recipient list, I wasn't sure who of C++ and x86
>>> maintainers should approve this]
>>> 
>>> Hi,
>>> 
>>> 29_atomics/atomic/65913.cc test in libstdc++ is a runtime test that only
>>> rely on atomic and gnu++11 support. Therefore I propose to require
>>> atomic-builtins instead of an x86 (32 or 64 bits) target.
>>> 
>>> ChangeLog entry is as follows:
>>> 
>>> 2016-05-19  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>> 
>>>        * testsuite/29_atomics/atomic/65913.cc: Require atomic-builtins
>>> 
>>> rather than specific target.
>>> 
>>> 
>>> Patch is in attachment.
>>> 
>>> 
>>> Is this ok for trunk?
>>> 
>>> Best regards,
>>> 
>>> Thomas
Jonathan Wakely June 29, 2016, 8:58 p.m. UTC | #3
On 29/06/16 13:49 -0700, Mike Stump wrote:
>Please include the libstdc++ list, they don't all read the other list.

And the documentation clearly says (in two places) that all libstdc++
patches must go to the libstdc++ list.
Jonathan Wakely June 29, 2016, 9:04 p.m. UTC | #4
On 29/06/16 13:49 -0700, Mike Stump wrote:
>>> On Thursday 26 May 2016 14:00:55 Thomas Preudhomme wrote:
>>>> [Sorry for the large recipient list, I wasn't sure who of C++ and x86
>>>> maintainers should approve this]
>>>>
>>>> Hi,
>>>>
>>>> 29_atomics/atomic/65913.cc test in libstdc++ is a runtime test that only
>>>> rely on atomic and gnu++11 support. Therefore I propose to require
>>>> atomic-builtins instead of an x86 (32 or 64 bits) target.
>>>>
>>>> ChangeLog entry is as follows:
>>>>
>>>> 2016-05-19  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>>>
>>>>        * testsuite/29_atomics/atomic/65913.cc: Require atomic-builtins
>>>>
>>>> rather than specific target.
>>>>
>>>>
>>>> Patch is in attachment.

The original mail with the patch is:
https://gcc.gnu.org/ml/gcc-patches/2016-06/msg00167.html

>>>>
>>>> Is this ok for trunk?

Yes, any target that passes dg-require-atomic-builtins should be able
to do that test without libatomic, so OK for trunk.
Thomas Preudhomme June 30, 2016, 2:18 p.m. UTC | #5
On Wednesday 29 June 2016 21:58:40 Jonathan Wakely wrote:
> On 29/06/16 13:49 -0700, Mike Stump wrote:
> >Please include the libstdc++ list, they don't all read the other list.
> 
> And the documentation clearly says (in two places) that all libstdc++
> patches must go to the libstdc++ list.

Oops my apologize, I did not think of libstdc++ has a separate component than 
GCC. I'll keep that in mind for next time.

Committed now.

Best regards,

Thomas
diff mbox

Patch

diff --git a/libstdc++-v3/testsuite/29_atomics/atomic/65913.cc b/libstdc++-v3/testsuite/29_atomics/atomic/65913.cc
index 713ef42d03cb9f7c1e691995df2d0943e24036c3..32a58ec991b41c74aafab84deed2c543d72505f5 100644
--- a/libstdc++-v3/testsuite/29_atomics/atomic/65913.cc
+++ b/libstdc++-v3/testsuite/29_atomics/atomic/65913.cc
@@ -15,7 +15,8 @@ 
 // with this library; see the file COPYING3.  If not see
 // <http://www.gnu.org/licenses/>.
 
-// { dg-do run { target x86_64-*-linux* powerpc*-*-linux* } }
+// { dg-do run }
+// { dg-require-atomic-builtins "" }
 // { dg-options "-std=gnu++11 -O0" }
 
 #include <atomic>