On 02/09/2012 03:24 PM, Benjamin Kosnik wrote:
> This is the rest of 51798, completing the conversion to C++11 atomics
> in libstdc++. This is now a complete transition, modulo documentation
> which I plan to finish as a separate patch once I am back from the ISO
> C++ meeting.
>
> tested x86_64/linux
>
> -benjamin
>
>
> 20120209-2.patch
>
>
> 2012-02-09 Benjamin Kosnik <b...@redhat.com>
> Jonathan Wakely <jwakely....@gmail.com>
>
> PR libstdc++/51798 continued.
> * acinclude.m4 (GLIBCXX_ENABLE_ATOMIC_BUILTINS): Use __atomic_*
> builtins instead of __sync_* builtins for atomic functionality.
> * include/bits/shared_ptr_base.h: Same.
> * include/parallel/compatibility.h: Same.
> * include/profile/impl/profiler_state.h: Same.
> * include/tr1/shared_ptr.h: Same.
> * libsupc++/eh_ptr.cc: Same.
> * libsupc++/eh_throw.cc: Same.
> * libsupc++/eh_tm.cc: Same.
> * libsupc++/guard.cc: Same.
> * configure: Regenerated.
> * testsuite/20_util/shared_ptr/cons/43820_neg.cc: Adjust line numbers.
> * testsuite/tr1/2_general_utilities/shared_ptr/cons/43820_neg.cc: Same.
The patch uses the weak version of compare_exchange universally, which
is incorrect in a number of cases. You wouldn't see this on x86_64;
you'd have to use a ll/sc target such as powerpc.
In addition to changing several uses to strong compare_exchange, I also
optimize the idiom
do
{
var = *m;
newval = ...;
}
while (!atomic_compare_exchange(m, &var, newval, ...));
With the new builtins, VAR is updated with the current value of the
memory (regardless of the weak setting), so the initial read from *M
can be hoisted outside the loop.
Ok?
r~
* include/bits/shared_ptr_base.h
(_Sp_counted_base<_S_atomic>::_M_add_ref_lock): Hoist initial load
outside compare_exchange loop.
* include/tr1/shared_ptr.h: Same.
* include/parallel/compatibility.h (__compare_and_swap_32): Use strong
version of compare_exchange.
(__compare_and_swap_64): Same.
* include/profile/impl/profiler_state.h (__gnu_profile::__turn): Same.
* libsupc++/guard.cc (__cxa_guard_acquire): Same.
diff --git a/libstdc++-v3/include/bits/shared_ptr_base.h
b/libstdc++-v3/include/bits/shared_ptr_base.h
index ebdc7ed..c48c18e 100644
--- a/libstdc++-v3/include/bits/shared_ptr_base.h
+++ b/libstdc++-v3/include/bits/shared_ptr_base.h
@@ -236,13 +236,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_M_add_ref_lock()
{
// Perform lock-free add-if-not-zero operation.
- _Atomic_word __count;
+ _Atomic_word __count = _M_use_count;
do
{
- __count = _M_use_count;
if (__count == 0)
__throw_bad_weak_ptr();
-
// Replace the current counter value with the old value + 1, as
// long as it's not changed meanwhile.
}
diff --git a/libstdc++-v3/include/parallel/compatibility.h
b/libstdc++-v3/include/parallel/compatibility.h
index 460345e..8a65c9e 100644
--- a/libstdc++-v3/include/parallel/compatibility.h
+++ b/libstdc++-v3/include/parallel/compatibility.h
@@ -252,8 +252,9 @@ namespace __gnu_parallel
__replacement, __comparand)
== __comparand;
#elif defined(__GNUC__)
- return __atomic_compare_exchange_n(__ptr, &__comparand, __replacement,
true,
- __ATOMIC_ACQ_REL, __ATOMIC_RELAXED);
+ return __atomic_compare_exchange_n(__ptr, &__comparand, __replacement,
+ false, __ATOMIC_ACQ_REL,
+ __ATOMIC_ACQ_REL);
#elif defined(__SUNPRO_CC) && defined(__sparc)
return atomic_cas_32((volatile unsigned int*)__ptr, __comparand,
__replacement) == __comparand;
@@ -299,13 +300,15 @@ namespace __gnu_parallel
#endif
#elif defined(__GNUC__) && defined(__x86_64)
- return __atomic_compare_exchange_n(__ptr, &__comparand, __replacement,
true,
- __ATOMIC_ACQ_REL, __ATOMIC_RELAXED);
+ return __atomic_compare_exchange_n(__ptr, &__comparand, __replacement,
+ false, __ATOMIC_ACQ_REL,
+ __ATOMIC_ACQ_REL);
#elif defined(__GNUC__) && defined(__i386) && \
(defined(__i686) || defined(__pentium4) || defined(__athlon) \
|| defined(__k8) || defined(__core2))
- return __atomic_compare_exchange_n(__ptr, &__comparand, __replacement,
true,
- __ATOMIC_ACQ_REL, __ATOMIC_RELAXED);
+ return __atomic_compare_exchange_n(__ptr, &__comparand, __replacement,
+ false, __ATOMIC_ACQ_REL,
+ __ATOMIC_ACQ_REL);
#elif defined(__SUNPRO_CC) && defined(__sparc)
return atomic_cas_64((volatile unsigned long long*)__ptr,
__comparand, __replacement) == __comparand;
diff --git a/libstdc++-v3/include/profile/impl/profiler_state.h
b/libstdc++-v3/include/profile/impl/profiler_state.h
index 573aa0e..37d761f 100644
--- a/libstdc++-v3/include/profile/impl/profiler_state.h
+++ b/libstdc++-v3/include/profile/impl/profiler_state.h
@@ -48,8 +48,8 @@ namespace __gnu_profile
{
__state_type inv(__INVALID);
return __atomic_compare_exchange_n(&_GLIBCXX_PROFILE_DATA(__state),
- &inv, __s, true, __ATOMIC_ACQ_REL,
- __ATOMIC_RELAXED);
+ &inv, __s, false, __ATOMIC_ACQ_REL,
+ __ATOMIC_ACQ_REL);
}
inline bool
diff --git a/libstdc++-v3/include/tr1/shared_ptr.h
b/libstdc++-v3/include/tr1/shared_ptr.h
index 723e317..5a1eb03 100644
--- a/libstdc++-v3/include/tr1/shared_ptr.h
+++ b/libstdc++-v3/include/tr1/shared_ptr.h
@@ -237,13 +237,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_M_add_ref_lock()
{
// Perform lock-free add-if-not-zero operation.
- _Atomic_word __count;
+ _Atomic_word __count = _M_use_count;
do
{
- __count = _M_use_count;
if (__count == 0)
__throw_bad_weak_ptr();
-
// Replace the current counter value with the old value + 1, as
// long as it's not changed meanwhile.
}
diff --git a/libstdc++-v3/libsupc++/guard.cc b/libstdc++-v3/libsupc++/guard.cc
index b7b8d3f..3f39dce 100644
--- a/libstdc++-v3/libsupc++/guard.cc
+++ b/libstdc++-v3/libsupc++/guard.cc
@@ -251,8 +251,9 @@ namespace __cxxabiv1
while (1)
{
- if (__atomic_compare_exchange_n(gi, &expected, pending_bit, true,
- __ATOMIC_ACQ_REL, __ATOMIC_RELAXED))
+ if (__atomic_compare_exchange_n(gi, &expected, pending_bit, false,
+ __ATOMIC_ACQ_REL,
+ __ATOMIC_ACQ_REL))
{
// This thread should do the initialization.
return 1;
@@ -266,9 +267,9 @@ namespace __cxxabiv1
if (expected == pending_bit)
{
int newv = expected | waiting_bit;
- if (!__atomic_compare_exchange_n(gi, &expected, newv, true,
+ if (!__atomic_compare_exchange_n(gi, &expected, newv, false,
__ATOMIC_ACQ_REL,
- __ATOMIC_RELAXED))
+ __ATOMIC_ACQ_REL))
continue;
expected = newv;
