[PATCH] fix generic std::atomicT::compare_exchange_{weak,strong}

2013-07-26 Thread Nathan Froyd 
Compiling the test program:

#include atomic

enum x { a, b };

std::atomicx v;

bool test_strong()
{
  x expected = a;
  return v.compare_exchange_strong(expected, b, std::memory_order_acq_rel);
}

bool test_weak()
{
  x expected = a;
  return v.compare_exchange_weak(expected, b, std::memory_order_acq_rel);
}

results in mysterious errors:

In file included from /home/froydnj/mini-atomic-bug.cpp:1:0:
/usr/include/c++/4.7/atomic: In function ‘bool test_strong()’:
/usr/include/c++/4.7/atomic:259:69: error: invalid failure memory model for 
‘__atomic_compare_exchange’
/usr/include/c++/4.7/atomic: In function ‘bool test_weak()’:
/usr/include/c++/4.7/atomic:235:68: error: invalid failure memory model for 
‘__atomic_compare_exchange’

as the generic std::atomicT versions of compare_exchange_strong and
compare_exchange_weak do not call __cmpexch_failure_order.

This patch corrects that oversight.  Tested on x86_64-unknown-linux-gnu.

OK to commit to trunk and active branches?

-Nathan

* include/std/atomic (compare_exchange_weak, compare_exchange_strong):
Add call to __cmpexch_failure_order.

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index ac2cb45..a822d0f 100644
diff --git a/libstdc++-v3/include/std/atomic b/libstdc++-v3/include/std/atomic
index 813f574..2d66729 100644
--- a/libstdc++-v3/include/std/atomic
+++ b/libstdc++-v3/include/std/atomic
@@ -252,12 +252,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   bool
   compare_exchange_weak(_Tp __e, _Tp __i,
memory_order __m = memory_order_seq_cst) noexcept
-  { return compare_exchange_weak(__e, __i, __m, __m); }
+  { return compare_exchange_weak(__e, __i, __m,
+ __cmpexch_failure_order(__m)); }
 
   bool
   compare_exchange_weak(_Tp __e, _Tp __i,
 memory_order __m = memory_order_seq_cst) volatile noexcept
-  { return compare_exchange_weak(__e, __i, __m, __m); }
+  { return compare_exchange_weak(__e, __i, __m,
+ __cmpexch_failure_order(__m)); }
 
   bool
   compare_exchange_strong(_Tp __e, _Tp __i, memory_order __s, 
@@ -276,12 +278,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   bool
   compare_exchange_strong(_Tp __e, _Tp __i,
   memory_order __m = memory_order_seq_cst) noexcept
-  { return compare_exchange_strong(__e, __i, __m, __m); }
+  { return compare_exchange_strong(__e, __i, __m,
+   __cmpexch_failure_order(__m)); }
 
   bool
   compare_exchange_strong(_Tp __e, _Tp __i,
 memory_order __m = memory_order_seq_cst) volatile noexcept
-  { return compare_exchange_strong(__e, __i, __m, __m); }
+  { return compare_exchange_strong(__e, __i, __m,
+   __cmpexch_failure_order(__m)); }
 };

Re: [PATCH] fix generic std::atomicT::compare_exchange_{weak,strong}

2013-07-26 Thread Paolo Carlini 


Hi,

Nathan Froyd nfr...@mozilla.com ha scritto:
Compiling the test program:

#include atomic

enum x { a, b };

std::atomicx v;

bool test_strong()
{
  x expected = a;
return v.compare_exchange_strong(expected, b,
std::memory_order_acq_rel);
}

bool test_weak()
{
  x expected = a;
return v.compare_exchange_weak(expected, b, std::memory_order_acq_rel);
}

In any case, why not adding the testcase?

Paolo

Re: [PATCH] fix generic std::atomicT::compare_exchange_{weak,strong}

2013-07-26 Thread Nathan Froyd 
Sure, I can do that.  For maximum effectiveness, it'd be good to have it check 
the specializations for atomic, too.  Is there something in the libstdc++ 
testsuite for iterating template instantiations over a list of types, or do I 
have to roll the list myself?

Thanks,
-Nathan

- Original Message -
 
 
 Hi,
 
 Nathan Froyd nfr...@mozilla.com ha scritto:
 Compiling the test program:
 
 #include atomic
 
 enum x { a, b };
 
 std::atomicx v;
 
 bool test_strong()
 {
   x expected = a;
 return v.compare_exchange_strong(expected, b,
 std::memory_order_acq_rel);
 }
 
 bool test_weak()
 {
   x expected = a;
 return v.compare_exchange_weak(expected, b, std::memory_order_acq_rel);
 }
 
 In any case, why not adding the testcase?
 
 Paolo

Re: [PATCH] fix generic std::atomicT::compare_exchange_{weak,strong}

2013-07-26 Thread Paolo Carlini 

Hi,

On 07/26/2013 08:42 PM, Nathan Froyd wrote:

Sure, I can do that.  For maximum effectiveness, it'd be good to have it check the 
specializations for atomic, too.  Is there something in the libstdc++ 
testsuite for iterating template instantiations over a list of types, or do I have to 
roll the list myself?

testsuite/29_atomics already uses testsuite_common_types.h

Paolo.

Re: [PATCH] fix generic std::atomicT::compare_exchange_{weak,strong}

2013-07-26 Thread Nathan Froyd 
- Original Message -
 On 07/26/2013 08:42 PM, Nathan Froyd wrote:
  Sure, I can do that.  For maximum effectiveness, it'd be good to have it
  check the specializations for atomic, too.  Is there something in the
  libstdc++ testsuite for iterating template instantiations over a list of
  types, or do I have to roll the list myself?
 testsuite/29_atomics already uses testsuite_common_types.h

New patch, this time with tests.  Let me know if test placement, etc. need
adjusting.

Tested on x86_64-unknown-linux-gnu.  OK for commit to trunk and active branches?

-Nathan

* include/std/atomic (compare_exchange_weak, compare_exchange_strong):
Add call to __cmpexch_failure_order.
* testsuite/util/testsuite_common_types.h
(compare_exchange_order_lowering): New generator.
* testsuite/29_atomics/atomic/requirements/compare_exchange_lowering.cc:
New test.

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index ac2cb45..3b79d91 100644
diff --git a/libstdc++-v3/include/std/atomic b/libstdc++-v3/include/std/atomic
index 813f574..2d66729 100644
--- a/libstdc++-v3/include/std/atomic
+++ b/libstdc++-v3/include/std/atomic
@@ -252,12 +252,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   bool
   compare_exchange_weak(_Tp __e, _Tp __i,
memory_order __m = memory_order_seq_cst) noexcept
-  { return compare_exchange_weak(__e, __i, __m, __m); }
+  { return compare_exchange_weak(__e, __i, __m,
+ __cmpexch_failure_order(__m)); }
 
   bool
   compare_exchange_weak(_Tp __e, _Tp __i,
 memory_order __m = memory_order_seq_cst) volatile noexcept
-  { return compare_exchange_weak(__e, __i, __m, __m); }
+  { return compare_exchange_weak(__e, __i, __m,
+ __cmpexch_failure_order(__m)); }
 
   bool
   compare_exchange_strong(_Tp __e, _Tp __i, memory_order __s, 
@@ -276,12 +278,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   bool
   compare_exchange_strong(_Tp __e, _Tp __i,
   memory_order __m = memory_order_seq_cst) noexcept
-  { return compare_exchange_strong(__e, __i, __m, __m); }
+  { return compare_exchange_strong(__e, __i, __m,
+   __cmpexch_failure_order(__m)); }
 
   bool
   compare_exchange_strong(_Tp __e, _Tp __i,
 memory_order __m = memory_order_seq_cst) volatile noexcept
-  { return compare_exchange_strong(__e, __i, __m, __m); }
+  { return compare_exchange_strong(__e, __i, __m,
+   __cmpexch_failure_order(__m)); }
 };
 
 
diff --git 
a/libstdc++-v3/testsuite/29_atomics/atomic/requirements/compare_exchange_lowering.cc
 
b/libstdc++-v3/testsuite/29_atomics/atomic/requirements/compare_exchange_lowering.cc
new file mode 100644
index 000..75e7406
--- /dev/null
+++ 
b/libstdc++-v3/testsuite/29_atomics/atomic/requirements/compare_exchange_lowering.cc
@@ -0,0 +1,65 @@
+// { dg-options -std=gnu++0x }
+// { dg-do compile }
+
+// Copyright (C) 2008-2013 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// http://www.gnu.org/licenses/.
+
+#include atomic
+#include testsuite_common_types.h
+
+#define TEST_ALL_ORDERS()   \
+  do {  \
+ORDER_TEST(std::memory_order_relaxed);  \
+ORDER_TEST(std::memory_order_consume);  \
+ORDER_TEST(std::memory_order_acquire);  \
+ORDER_TEST(std::memory_order_release);  \
+ORDER_TEST(std::memory_order_acq_rel);  \
+ORDER_TEST(std::memory_order_seq_cst);  \
+  } while(0)
+  
+void test01()
+{
+#define ORDER_TEST(ORDER)   \
+  do {  \
+__gnu_test::compare_exchange_order_loweringORDER test;\
+__gnu_cxx::typelist::apply_generator(test,  \
+ __gnu_test::integral_types::type()); \
+  } while (0);
+  TEST_ALL_ORDERS();
+#undef ORDER_TEST
+
+  enum e { a, b, c };
+#define ORDER_TEST(ORDER)   \
+  do {  \
+std::atomice x(a);

Re: [PATCH] fix generic std::atomicT::compare_exchange_{weak,strong}

2013-07-26 Thread Paolo Carlini 

Hi,

On 07/27/2013 01:07 AM, Nathan Froyd wrote:

- Original Message -

On 07/26/2013 08:42 PM, Nathan Froyd wrote:

Sure, I can do that.  For maximum effectiveness, it'd be good to have it
check the specializations for atomic, too.  Is there something in the
libstdc++ testsuite for iterating template instantiations over a list of
types, or do I have to roll the list myself?

testsuite/29_atomics already uses testsuite_common_types.h

New patch, this time with tests.  Let me know if test placement, etc. need
adjusting.
Patch looks great, Ok to commit. Minor nits: in the testcase we want 
only 2013 as Copyright year and let's standardize on -std=gnu++11.


Thanks,
Paolo.