From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id 4EF6A386F43B for ; Tue, 3 Nov 2020 20:04:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 4EF6A386F43B Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-36-UxRfZizuPzeUqkmHwoVJVg-1; Tue, 03 Nov 2020 15:04:20 -0500 X-MC-Unique: UxRfZizuPzeUqkmHwoVJVg-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 845A0809DFD; Tue, 3 Nov 2020 20:04:19 +0000 (UTC) Received: from localhost (unknown [10.33.36.7]) by smtp.corp.redhat.com (Postfix) with ESMTP id 107165D9CC; Tue, 3 Nov 2020 20:04:18 +0000 (UTC) Date: Tue, 3 Nov 2020 20:04:18 +0000 From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: Re: [committed] libstdc++: Rewrite std::call_once to use futexes [PR 66146] Message-ID: <20201103200418.GW503596@redhat.com> References: <20201103184534.GA3112738@redhat.com> MIME-Version: 1.0 In-Reply-To: <20201103184534.GA3112738@redhat.com> X-Clacks-Overhead: GNU Terry Pratchett X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/mixed; boundary="k+G3HLlWI7eRTl+h" Content-Disposition: inline X-Spam-Status: No, score=-14.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=unavailable autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 03 Nov 2020 20:04:25 -0000 --k+G3HLlWI7eRTl+h Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline This is a minor refactoring for readability. Tested x86_64-linux and powerpc-aix. Committed to trunk. --k+G3HLlWI7eRTl+h Content-Type: text/x-patch; charset=us-ascii Content-Disposition: attachment; filename="patch.txt" commit 9f925f3b198e210e0d124a3c69fae034f429942f Author: Jonathan Wakely Date: Tue Nov 3 19:42:07 2020 libstdc++: Refactor std::call_once internals This separates the definition of std::__call_proxy into two funcions, one for TLS and one for non-TLS, to make them easier to read. It also replaces the __get_once_functor_lock_ptr() internal helper with a new set_lock_ptr(unique_lock*) function so that __once_proxy doesn't need to call it twice. libstdc++-v3/ChangeLog: * src/c++11/mutex.cc [_GLIBCXX_HAVE_TLS] (__once_proxy): Define separately for TLS targets. [!_GLIBCXX_HAVE_TLS] (__get_once_functor_lock_ptr): Replace with ... (set_lock_ptr): ... this. Set new value and return previous value. [!_GLIBCXX_HAVE_TLS] (__set_once_functor_lock_ptr): Adjust to use set_lock_ptr. [!_GLIBCXX_HAVE_TLS] (__once_proxy): Likewise. diff --git a/libstdc++-v3/src/c++11/mutex.cc b/libstdc++-v3/src/c++11/mutex.cc index 286f77f9a454..4d42bed8ecc9 100644 --- a/libstdc++-v3/src/c++11/mutex.cc +++ b/libstdc++-v3/src/c++11/mutex.cc @@ -84,18 +84,6 @@ std::once_flag::_M_finish(bool returning) noexcept #endif // ! FUTEX -#ifndef _GLIBCXX_HAVE_TLS -namespace -{ - inline std::unique_lock*& - __get_once_functor_lock_ptr() - { - static std::unique_lock* __once_functor_lock_ptr = 0; - return __once_functor_lock_ptr; - } -} -#endif - namespace std _GLIBCXX_VISIBILITY(default) { _GLIBCXX_BEGIN_NAMESPACE_VERSION @@ -103,9 +91,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #ifdef _GLIBCXX_HAVE_TLS __thread void* __once_callable; __thread void (*__once_call)(); -#else + + extern "C" void __once_proxy() + { + // The caller stored a function pointer in __once_call. If it requires + // any state, it gets it from __once_callable. + __once_call(); + } + +#else // ! TLS + // Explicit instantiation due to -fno-implicit-instantiation. template class function; + function __once_functor; mutex& @@ -115,11 +113,22 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION return once_mutex; } +namespace +{ + // Store ptr in a global variable and return the previous value. + inline unique_lock* + set_lock_ptr(unique_lock* ptr) + { + static unique_lock* __once_functor_lock_ptr = nullptr; + return std::__exchange(__once_functor_lock_ptr, ptr); + } +} + // code linked against ABI 3.4.12 and later uses this void __set_once_functor_lock_ptr(unique_lock* __ptr) { - __get_once_functor_lock_ptr() = __ptr; + (void) set_lock_ptr(__ptr); } // unsafe - retained for compatibility with ABI 3.4.11 @@ -129,26 +138,27 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION static unique_lock once_functor_lock(__get_once_mutex(), defer_lock); return once_functor_lock; } -#endif - extern "C" + // This is called via pthread_once while __get_once_mutex() is locked. + extern "C" void + __once_proxy() { - void __once_proxy() + // Get the callable out of the global functor. + function callable = std::move(__once_functor); + + // Then unlock the global mutex + if (unique_lock* lock = set_lock_ptr(nullptr)) { -#ifndef _GLIBCXX_HAVE_TLS - function __once_call = std::move(__once_functor); - if (unique_lock* __lock = __get_once_functor_lock_ptr()) - { - // caller is using new ABI and provided lock ptr - __get_once_functor_lock_ptr() = 0; - __lock->unlock(); - } - else - __get_once_functor_lock().unlock(); // global lock -#endif - __once_call(); + // Caller is using the new ABI and provided a pointer to its lock. + lock->unlock(); } + else + __get_once_functor_lock().unlock(); // global lock + + // Finally, invoke the callable. + callable(); } +#endif // ! TLS _GLIBCXX_END_NAMESPACE_VERSION } // namespace std --k+G3HLlWI7eRTl+h--