From patchwork Sat Feb 11 00:56:18 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Henderson X-Patchwork-Id: 140747 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 1E0A6B6F9F for ; Sat, 11 Feb 2012 11:57:06 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1329526627; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:Message-ID:Date:From:User-Agent:MIME-Version:To:CC: Subject:References:In-Reply-To:Content-Type:Mailing-List: Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:Sender:Delivered-To; bh=vfLtSJO5VmWTi7nPVpSriuIihSw=; b=J+kd1mKSBA+sYFrfHPlratJPiUEaBRPuj69XkWAdrSMLa05I4HaPxLbkhEW60M ajdKH6k3nNaSdRsS15s+rCGQYCpyLUNM05BaY1JSePAjtNxJ+X4+fC9Tnz3PbVdE 8lWXpd2011NOSLPCJWfr9XzxabN+NsbFDEe98xEXfTrJs= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject:References:In-Reply-To:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=W7SWB0mDwggK00LrDNHsoH2M/RVCI30jQyLSDao1OXC945B5ItfvKiyn2Uw9JJ coRxzzPT5JRDOVLrKC1IYIGu7rEKKprw/felZ7gRrHjYfY2TyXprk/nNnIwamPGd QmOqqqd7u1Ot0PMvutu6ZanWaZwmt4OyE8Y17r0QtBmec=; Received: (qmail 10287 invoked by alias); 11 Feb 2012 00:56:57 -0000 Received: (qmail 10273 invoked by uid 22791); 11 Feb 2012 00:56:55 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, TW_CX X-Spam-Check-By: sourceware.org Received: from mail-qy0-f177.google.com (HELO mail-qy0-f177.google.com) (209.85.216.177) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 11 Feb 2012 00:56:21 +0000 Received: by qcsu28 with SMTP id u28so2380530qcs.22 for ; Fri, 10 Feb 2012 16:56:21 -0800 (PST) Received: by 10.229.135.146 with SMTP id n18mr5336209qct.72.1328921781032; Fri, 10 Feb 2012 16:56:21 -0800 (PST) Received: from anchor.twiddle.home ([173.160.232.49]) by mx.google.com with ESMTPS id fd1sm16437523qab.1.2012.02.10.16.56.19 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 10 Feb 2012 16:56:20 -0800 (PST) Message-ID: <4F35BCB2.5050806@redhat.com> Date: Fri, 10 Feb 2012 16:56:18 -0800 From: Richard Henderson User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0) Gecko/20120131 Thunderbird/10.0 MIME-Version: 1.0 To: Benjamin Kosnik CC: gcc-patches@gcc.gnu.org, libstdc++@gcc.gnu.org, jakub@redhat.com Subject: Re: [v3] libstdc++/51798 References: <20120209152408.7592e6e7@shotwell> In-Reply-To: <20120209152408.7592e6e7@shotwell> X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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 > Jonathan Wakely > > 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;