From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2181) id 8B58B385843A; Thu, 5 Jan 2023 00:51:56 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8B58B385843A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1672879916; bh=F57bZZlNpNUSg3uA+ibsnxqJYCQHK4REw93WrdEA/Gw=; h=From:To:Subject:Date:From; b=QKXsv3lzQYDTHygdt4w5VRQ+LGVSdQ2WG1moM/iZmKLcxYZRNnK8kW6q1tUeOT2Bv 6mv8THI8h4Nrn3zjGI+aQYRk0zNV1tNc0X7Mi4ObezxDzRpL6fpPUB6N5izvFyg/z0 oQGaXxBBoewQbnjcQKRi/T/GBUmNTfjyzN6DvP1E= MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Jonathan Wakely To: gcc-cvs@gcc.gnu.org, libstdc++-cvs@gcc.gnu.org Subject: [gcc r13-5003] libstdc++: Only use std::atomic if lock free [PR108228] X-Act-Checkin: gcc X-Git-Author: Jonathan Wakely X-Git-Refname: refs/heads/master X-Git-Oldrev: e36e57b032b2d70eaa1294d5921e4fd8ce12a74d X-Git-Newrev: b1ad748754401613b5cf8e5d46b38ad1ee49d07a Message-Id: <20230105005156.8B58B385843A@sourceware.org> Date: Thu, 5 Jan 2023 00:51:56 +0000 (GMT) List-Id: https://gcc.gnu.org/g:b1ad748754401613b5cf8e5d46b38ad1ee49d07a commit r13-5003-gb1ad748754401613b5cf8e5d46b38ad1ee49d07a Author: Jonathan Wakely Date: Wed Jan 4 16:45:14 2023 +0000 libstdc++: Only use std::atomic if lock free [PR108228] This fixes linker errors for hppa-hp-hpux11.11 due to an undefined weak symbol and the use of atomic operations that require libatomic. The weak symbol can simply be defined, which we already do for darwin. The std::atomic<_Node*> is only an optimization, so can be avoided for targets where the underlying atomic ops aren't available without help from libatomic. The accesses to the std::atomic<_Node*> can be abstracted behind a new API for getting and setting the cached value, and then the atomics can be used conditionally. libstdc++-v3/ChangeLog: PR libstdc++/108228 PR libstdc++/108235 * config/abi/pre/gnu.ver: Move zoneinfo_dir_override export to the latest symbol version. * src/c++20/tzdb.cc (USE_ATOMIC_SHARED_PTR): Define to 0 if atomic<_Node*> is not always lock free. (USE_ATOMIC_LIST_HEAD): New macro. [__hpux__] (__gnu_cxx::zoneinfo_dir_override()): Provide definition of weak symbol. (tzdb_list::_Node::_S_head): Rename to _S_head_cache. (tzdb_list::_Node::_S_list_head): New function for accessing list head efficiently. (tzdb_list::_Node::_S_cache_list_head): New function for updating _S_list_head. Diff: --- libstdc++-v3/config/abi/pre/gnu.ver | 4 +- libstdc++-v3/src/c++20/tzdb.cc | 97 ++++++++++++++++++++++++------------- 2 files changed, 64 insertions(+), 37 deletions(-) diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver index dc46f670b78..a5559a1c374 100644 --- a/libstdc++-v3/config/abi/pre/gnu.ver +++ b/libstdc++-v3/config/abi/pre/gnu.ver @@ -1104,9 +1104,6 @@ GLIBCXX_3.4 { # std::uncaught_exception() _ZSt18uncaught_exceptionv; - # __gnu_cxx::zoneinfo_dir_override() - _ZN9__gnu_cxx21zoneinfo_dir_overrideEv; - # DO NOT DELETE THIS LINE. Port-specific symbols, if any, will be here. local: @@ -2505,6 +2502,7 @@ GLIBCXX_3.4.31 { _ZNKSt6chrono9tzdb_list14const_iteratordeEv; _ZNSt6chrono9tzdb_list14const_iteratorppEv; _ZNSt6chrono9tzdb_list14const_iteratorppEi; + _ZN9__gnu_cxx21zoneinfo_dir_overrideEv; } GLIBCXX_3.4.30; diff --git a/libstdc++-v3/src/c++20/tzdb.cc b/libstdc++-v3/src/c++20/tzdb.cc index 2a4e213d3d9..6772517d55a 100644 --- a/libstdc++-v3/src/c++20/tzdb.cc +++ b/libstdc++-v3/src/c++20/tzdb.cc @@ -34,14 +34,22 @@ #include // mutex #include // filesystem::read_symlink -#ifdef __GTHREADS -# if _WIN32 +#ifndef __GTHREADS +# define USE_ATOMIC_SHARED_PTR 0 +#elif _WIN32 // std::mutex cannot be constinit, so Windows must use atomic>. -# define USE_ATOMIC_SHARED_PTR 1 -# else +# define USE_ATOMIC_SHARED_PTR 1 +#elif ATOMIC_POINTER_LOCK_FREE < 2 +# define USE_ATOMIC_SHARED_PTR 0 +#else // TODO benchmark atomic> vs mutex. -# define USE_ATOMIC_SHARED_PTR 1 -# endif +# define USE_ATOMIC_SHARED_PTR 1 +#endif + +#if defined __GTHREADS && ATOMIC_POINTER_LOCK_FREE == 2 +# define USE_ATOMIC_LIST_HEAD 1 +#else +# define USE_ATOMIC_LIST_HEAD 0 #endif #if ! __cpp_constinit @@ -64,7 +72,7 @@ namespace __gnu_cxx #else [[gnu::weak]] const char* zoneinfo_dir_override(); -#ifdef __APPLE__ +#if defined(__APPLE__) || defined(__hpux__) // Need a weak definition for Mach-O. [[gnu::weak]] const char* zoneinfo_dir_override() { return _GLIBCXX_ZONEINFO_DIR; } @@ -76,6 +84,15 @@ namespace std::chrono { namespace { +#if ! USE_ATOMIC_SHARED_PTR +#ifndef __GTHREADS + // Dummy no-op mutex type for single-threaded targets. + struct mutex { void lock() { } void unlock() { } }; +#endif + /// XXX std::mutex::mutex() not constexpr on Windows, so can't be constinit + constinit mutex list_mutex; +#endif + struct Rule; } @@ -103,8 +120,29 @@ namespace std::chrono // This is the owning reference to the first tzdb in the list. static head_ptr _S_head_owner; +#if USE_ATOMIC_LIST_HEAD // Lock-free access to the head of the list. - static atomic<_Node*> _S_head; + static atomic<_Node*> _S_head_cache; + + static _Node* + _S_list_head(memory_order ord) noexcept + { return _S_head_cache.load(ord); } + + static void + _S_cache_list_head(_Node* new_head) noexcept + { _S_head_cache = new_head; } +#else + static _Node* + _S_list_head(memory_order) + { + lock_guard l(list_mutex); + return _S_head_owner.get(); + } + + static void + _S_cache_list_head(_Node*) noexcept + { } +#endif static const tzdb& _S_init_tzdb(); static const tzdb& _S_replace_head(shared_ptr<_Node>, shared_ptr<_Node>); @@ -122,8 +160,10 @@ namespace std::chrono // Shared pointer to the first Node in the list. constinit tzdb_list::_Node::head_ptr tzdb_list::_Node::_S_head_owner{nullptr}; +#if USE_ATOMIC_LIST_HEAD // Lock-free access to the first Node in the list. - constinit atomic tzdb_list::_Node::_S_head{nullptr}; + constinit atomic tzdb_list::_Node::_S_head_cache{nullptr}; +#endif // The data structures defined in this file (Rule, on_day, at_time etc.) // are used to represent the information parsed from the tzdata.zi file @@ -135,15 +175,6 @@ namespace std::chrono namespace { -#if ! USE_ATOMIC_SHARED_PTR -#ifndef __GTHREADS - // Dummy no-op mutex type for single-threaded targets. - struct mutex { void lock() { } void unlock() { } }; -#endif - /// XXX std::mutex::mutex() not constexpr on Windows, so can't be constinit - constinit mutex list_mutex; -#endif - // Used for reading a possibly-quoted string from a stream. struct quoted { @@ -1101,30 +1132,28 @@ namespace std::chrono tzdb_list::_Node::_S_replace_head(shared_ptr<_Node> curr [[maybe_unused]], shared_ptr<_Node> new_head) { + _Node* new_head_ptr = new_head.get(); #if USE_ATOMIC_SHARED_PTR - new_head->next = curr; + new_head_ptr->next = curr; while (!_S_head_owner.compare_exchange_strong(curr, new_head)) { - if (curr->db.version == new_head->db.version) + if (curr->db.version == new_head_ptr->db.version) return curr->db; - new_head->next = curr; + new_head_ptr->next = curr; } - // XXX small window here where _S_head still points to previous tzdb. - _Node::_S_head = new_head.get(); - return new_head->db; + // XXX small window here where _S_head_cache still points to previous tzdb. #else lock_guard l(list_mutex); - if (const _Node* h = _S_head) + if (const _Node* h = _S_head_owner.get()) { - if (h->db.version == new_head->db.version) + if (h->db.version == new_head_ptr->db.version) return h->db; - new_head->next = _S_head_owner; + new_head_ptr->next = _S_head_owner; } - auto* pnode = new_head.get(); _S_head_owner = std::move(new_head); - _S_head = pnode; - return pnode->db; #endif + _S_cache_list_head(new_head_ptr); + return new_head_ptr->db; } // Called to populate the list for the first time. If reload_tzdb() fails, @@ -1207,7 +1236,7 @@ namespace std::chrono get_tzdb_list() { using Node = tzdb_list::_Node; - if (Node::_S_head.load(memory_order::acquire) == nullptr) [[unlikely]] + if (Node::_S_list_head(memory_order::acquire) == nullptr) [[unlikely]] Node::_S_init_tzdb(); // populates list return Node::_S_the_list; } @@ -1217,7 +1246,7 @@ namespace std::chrono get_tzdb() { using Node = tzdb_list::_Node; - if (auto* __p = Node::_S_head.load(memory_order::acquire)) [[likely]] + if (auto* __p = Node::_S_list_head(memory_order::acquire)) [[likely]] return __p->db; return Node::_S_init_tzdb(); // populates list } @@ -1236,7 +1265,7 @@ namespace std::chrono if (head != nullptr && head->db.version == version) return head->db; #else - if (Node::_S_head.load(memory_order::relaxed) != nullptr) [[likely]] + if (Node::_S_list_head(memory_order::relaxed) != nullptr) [[likely]] { lock_guard l(list_mutex); const tzdb& current = Node::_S_head_owner->db; @@ -1346,7 +1375,7 @@ namespace std::chrono const tzdb& tzdb_list::front() const noexcept { - return _Node::_S_head.load()->db; + return _Node::_S_list_head(memory_order::seq_cst)->db; } // Implementation of std::chrono::tzdb_list::begin().