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 [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id AB22D385B530 for ; Thu, 5 Jan 2023 00:52:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AB22D385B530 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1672879962; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=PHdku6Qs55LhvY+zTV4bu7kpU2bfgg20GzUB6toSQXc=; b=SV8BOw+LQDRzbGinFJ0+2Paucd10A911V+aslu1xGEbSwLIJzEOX5ZeJoMiv6wKn5ONw9g BnUNNGGQTMniY74gctzB1WvkO2VQLdGK88AHF8NDASknMDtqZXPoLhn+1P+7w+YcxS80yv GK6wLqY8EPJK2omqvIz1TLfJN3SCtkA= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-280-_sIhqnMGNVeoVsHYh-8P9w-1; Wed, 04 Jan 2023 19:52:41 -0500 X-MC-Unique: _sIhqnMGNVeoVsHYh-8P9w-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E7ACD85A588; Thu, 5 Jan 2023 00:52:40 +0000 (UTC) Received: from localhost (unknown [10.33.36.242]) by smtp.corp.redhat.com (Postfix) with ESMTP id AC5B3C15BA0; Thu, 5 Jan 2023 00:52:40 +0000 (UTC) From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [committed] libstdc++: Only use std::atomic if lock free [PR108228] Date: Thu, 5 Jan 2023 00:52:40 +0000 Message-Id: <20230105005240.140165-1-jwakely@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.8 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.8 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_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Tested x86_64-linux. Pushed to trunk. -- >8 -- 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. --- 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(). -- 2.39.0