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 ESMTP id 05822388E83F for ; Wed, 21 Apr 2021 12:12:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 05822388E83F 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-334-VCayGuKANNaghYVCVmSU3w-1; Wed, 21 Apr 2021 08:12:20 -0400 X-MC-Unique: VCayGuKANNaghYVCVmSU3w-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 B0F9B10CE782; Wed, 21 Apr 2021 12:12:19 +0000 (UTC) Received: from localhost (unknown [10.33.36.164]) by smtp.corp.redhat.com (Postfix) with ESMTP id 17F8D5DDAD; Wed, 21 Apr 2021 12:12:18 +0000 (UTC) Date: Wed, 21 Apr 2021 13:12:18 +0100 From: Jonathan Wakely To: Thomas Rodgers Cc: David Edelsohn , Jakub Jelinek , gcc-patches@gcc.gnu.org, libstdc++@gcc.gnu.org Subject: Re: GCC 11.1 Release Candidate available from gcc.gnu.org Message-ID: <20210421121218.GV3008@redhat.com> References: <20210420152439.GR1179226@tucnak> <6ac731c557415b46b04613d9b908df2f@appliantology.com> <45de03629a536c0000c9b47bb79d6601@appliantology.com> <20210421113804.GU3008@redhat.com> MIME-Version: 1.0 In-Reply-To: <20210421113804.GU3008@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="Zlo2GrDIozf6aQFY" Content-Disposition: inline X-Spam-Status: No, score=-14.0 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_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham 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: Wed, 21 Apr 2021 12:12:33 -0000 --Zlo2GrDIozf6aQFY Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline On 21/04/21 12:38 +0100, Jonathan Wakely wrote: >On 20/04/21 22:12 -0700, Thomas Rodgers wrote: >>@@ -86,6 +88,24 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> } >> } >> >>+ _GLIBCXX_ALWAYS_INLINE bool >>+ _M_try_acquire() noexcept >>+ { >>+ for (;;) >>+ { >>+ auto __err = sem_trywait(&_M_semaphore); >>+ if (__err && (errno == EINTR)) >>+ continue; >>+ else if (__err && (errno == EAGAIN)) >>+ return false; >>+ else if (__err) >>+ std::terminate(); >>+ else >>+ break; >>+ } >>+ return true; >>+ } >>+ >> _GLIBCXX_ALWAYS_INLINE void >> _M_release(std::ptrdiff_t __update) noexcept >> { > >Please just commit this part to trunk and gcc-11, not the macro >renaming (as that's been fixed by Jakub already). I think on trunk I'd prefer to do the attached. WDYT? --Zlo2GrDIozf6aQFY Content-Type: text/x-patch; charset=us-ascii Content-Disposition: attachment; filename="patch.txt" commit 07241d79b6720d4f392d5a8ba6e9c21d2801c8c7 Author: Jonathan Wakely Date: Wed Apr 21 13:04:05 2021 libstdc++: Streamline implementation [PR 100164] This adds the missing _M_try_acquire member function so that __platform_semaphore works. Also adjust the implementation so that __platform_semaphore is not defined unless we're going to use it, which avoids including (and polluting he global namespace) when we don't need it. Also rename the _GLIBCXX_REQUIRE_POSIX_SEMAPHORE macro to _GLIBCXX_USE_POSIX_SEMAPHORE for consistency with the similar _GLIBCXX_USE_CXX11_ABI macro that can be used to request an alternative (ABI-changing) implementation. libstdc++-v3/ChangeLog: PR libstdc++/100164 * include/bits/semaphore_base.h: Only define at most one of __platform_semaphore and __atomic_semaphore. (__platform_semaphore::_M_try_acquire()): Add missing function. * include/std/semaphore: Do not define anything unless one of the implementations is available. * testsuite/30_threads/semaphore/try_acquire_posix.cc: Define macro to request POSIX semaphore implementation. Use standard API, not private implementation. diff --git a/libstdc++-v3/include/bits/semaphore_base.h b/libstdc++-v3/include/bits/semaphore_base.h index 7e3235d182e..80134d7fc4c 100644 --- a/libstdc++-v3/include/bits/semaphore_base.h +++ b/libstdc++-v3/include/bits/semaphore_base.h @@ -1,4 +1,4 @@ -// -*- C++ -*- header. +// std::counting_semaphore implementation -*- C++ -*- header. // Copyright (C) 2020-2021 Free Software Foundation, Inc. // @@ -32,25 +32,32 @@ #pragma GCC system_header -#include -#if __cpp_lib_atomic_wait -#include -#include -#endif // __cpp_lib_atomic_wait - -#ifdef _GLIBCXX_HAVE_POSIX_SEMAPHORE -# include -# include -#endif - #include #include +// Note: the _GLIBCXX_USE_POSIX_SEMAPHORE macro can be used to force the +// use of Posix semaphores (sem_t). Doing so however, alters the ABI. + +#ifndef _GLIBCXX_HAVE_POSIX_SEMAPHORE +# undef _GLIBCXX_USE_POSIX_SEMAPHORE +#endif + +#include +#if __cpp_lib_atomic_wait && ! _GLIBCXX_USE_POSIX_SEMAPHORE +# include +# include +#elif _GLIBCXX_HAVE_POSIX_SEMAPHORE +# include +# include +# undef _GLIBCXX_USE_POSIX_SEMAPHORE +# define _GLIBCXX_USE_POSIX_SEMAPHORE 1 +#endif + namespace std _GLIBCXX_VISIBILITY(default) { _GLIBCXX_BEGIN_NAMESPACE_VERSION -#ifdef _GLIBCXX_HAVE_POSIX_SEMAPHORE +#if _GLIBCXX_USE_POSIX_SEMAPHORE struct __platform_semaphore { using __clock_t = chrono::system_clock; @@ -76,23 +83,44 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { for (;;) { - auto __err = sem_wait(&_M_semaphore); - if (__err && (errno == EINTR)) - continue; - else if (__err) - std::terminate(); + if (auto __err = sem_wait(&_M_semaphore)) + { + if (errno == EINTR) + continue; + else + std::terminate(); + } else break; } } + _GLIBCXX_ALWAYS_INLINE bool + _M_try_acquire() noexcept + { + for (;;) + { + if (auto __err = sem_trywait(&_M_semaphore)) + { + if (errno == EINTR) + continue; + else if (errno == EAGAIN) + return false; + else + std::terminate(); + } + else + break; + } + return true; + } + _GLIBCXX_ALWAYS_INLINE void _M_release(std::ptrdiff_t __update) noexcept { for(; __update != 0; --__update) { - auto __err = sem_post(&_M_semaphore); - if (__err) + if (auto __err = sem_post(&_M_semaphore)) std::terminate(); } } @@ -162,9 +190,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION private: sem_t _M_semaphore; }; -#endif // _GLIBCXX_HAVE_POSIX_SEMAPHORE -#if __cpp_lib_atomic_wait + using __semaphore_impl = __platform_semaphore; + +#elif __cpp_lib_atomic_wait + struct __atomic_semaphore { static constexpr ptrdiff_t _S_max = __gnu_cxx::__int_traits::__max; @@ -245,19 +275,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION private: alignas(__detail::__platform_wait_alignment) - __detail::__platform_wait_t _M_counter; + __detail::__platform_wait_t _M_counter; }; -#endif // __cpp_lib_atomic_wait -// Note: the _GLIBCXX_REQUIRE_POSIX_SEMAPHORE macro can be used to force the -// use of Posix semaphores (sem_t). Doing so however, alters the ABI. -#if defined __cpp_lib_atomic_wait && !_GLIBCXX_REQUIRE_POSIX_SEMAPHORE using __semaphore_impl = __atomic_semaphore; -#elif _GLIBCXX_HAVE_POSIX_SEMAPHORE - using __semaphore_impl = __platform_semaphore; -#else -# error "No suitable semaphore implementation available" -#endif +#endif // __cpp_lib_atomic_wait _GLIBCXX_END_NAMESPACE_VERSION } // namespace std diff --git a/libstdc++-v3/include/std/semaphore b/libstdc++-v3/include/std/semaphore index a1560915d83..43abe8a9495 100644 --- a/libstdc++-v3/include/std/semaphore +++ b/libstdc++-v3/include/std/semaphore @@ -34,6 +34,7 @@ #if __cplusplus > 201703L #include +#if __cpp_lib_atomic_wait || _GLIBCXX_USE_POSIX_SEMAPHORE namespace std _GLIBCXX_VISIBILITY(default) { _GLIBCXX_BEGIN_NAMESPACE_VERSION @@ -89,5 +90,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _GLIBCXX_END_NAMESPACE_VERSION } // namespace +#endif // cpp_lib_atomic_wait || _GLIBCXX_USE_POSIX_SEMAPHORE #endif // C++20 #endif // _GLIBCXX_SEMAPHORE diff --git a/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_posix.cc b/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_posix.cc index 97e386f7b76..b18fb623ecc 100644 --- a/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_posix.cc +++ b/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_posix.cc @@ -20,30 +20,39 @@ // { dg-require-effective-target pthread } // { dg-require-gthreads "" } +// Request the use of POSIX semaphores +#define _GLIBCXX_USE_POSIX_SEMAPHORE 1 #include -#ifdef _GLIBCXX_HAVE_POSIX_SEMAPHORE + +#if __cpp_lib_semaphore +#if _GLIBCXX_USE_POSIX_SEMAPHORE +static_assert(sizeof(std::counting_semaphore<>) == sizeof(::sem_t)); +#endif + #include #include #include #include +using semaphore = std::counting_semaphore<>; + void test01() { using namespace std::chrono_literals; - std::__platform_semaphore s(2); - s._M_acquire(); + semaphore s(2); + s.acquire(); auto const dur = 250ms; { auto const t0 = std::chrono::steady_clock::now(); - VERIFY( s._M_try_acquire_for(dur) ); + VERIFY( s.try_acquire_for(dur) ); auto const diff = std::chrono::steady_clock::now() - t0; VERIFY( diff < dur ); } { auto const t0 = std::chrono::steady_clock::now(); - VERIFY( !s._M_try_acquire_for(dur) ); + VERIFY( !s.try_acquire_for(dur) ); auto const diff = std::chrono::steady_clock::now() - t0; VERIFY( diff >= dur ); } @@ -52,27 +61,27 @@ void test01() void test02() { using namespace std::chrono_literals; - std::__platform_semaphore s(1); + semaphore s(1); std::atomic a(0), b(0); std::thread t([&] { a.wait(0); auto const dur = 250ms; - VERIFY( !s._M_try_acquire_for(dur) ); + VERIFY( !s.try_acquire_for(dur) ); b++; b.notify_one(); a.wait(1); - VERIFY( s._M_try_acquire_for(dur) ); + VERIFY( s.try_acquire_for(dur) ); b++; b.notify_one(); }); t.detach(); - s._M_acquire(); + s.acquire(); a++; a.notify_one(); b.wait(0); - s._M_release(1); + s.release(1); a++; a.notify_one(); @@ -82,14 +91,14 @@ void test02() void test03() { using namespace std::chrono_literals; - std::__platform_semaphore s(2); - s._M_acquire(); + semaphore s(2); + s.acquire(); auto const dur = 250ms; { auto const at = std::chrono::system_clock::now() + dur; auto const t0 = std::chrono::steady_clock::now(); - VERIFY( s._M_try_acquire_until(at) ); + VERIFY( s.try_acquire_until(at) ); auto const diff = std::chrono::steady_clock::now() - t0; VERIFY( diff < dur ); } @@ -97,7 +106,7 @@ void test03() { auto const at = std::chrono::system_clock::now() + dur; auto const t0 = std::chrono::steady_clock::now(); - VERIFY( !s._M_try_acquire_until(at) ); + VERIFY( !s.try_acquire_until(at) ); auto const diff = std::chrono::steady_clock::now() - t0; VERIFY( diff >= dur ); } @@ -106,14 +115,14 @@ void test03() void test04() { using namespace std::chrono_literals; - std::__platform_semaphore s(1); + semaphore s(1); std::atomic a(0), b(0); std::thread t([&] { a.wait(0); auto const dur = 250ms; { auto const at = std::chrono::system_clock::now() + dur; - VERIFY( !s._M_try_acquire_until(at) ); + VERIFY( !s.try_acquire_until(at) ); b++; b.notify_one(); @@ -122,32 +131,31 @@ void test04() a.wait(1); { auto const at = std::chrono::system_clock::now() + dur; - VERIFY( s._M_try_acquire_until(at) ); + VERIFY( s.try_acquire_until(at) ); } b++; b.notify_one(); }); t.detach(); - s._M_acquire(); + s.acquire(); a++; a.notify_one(); b.wait(0); - s._M_release(1); + s.release(1); a++; a.notify_one(); b.wait(1); } -#endif int main() { -#ifdef _GLIBCXX_HAVE_POSIX_SEMAPHORE test01(); test02(); test03(); test04(); -#endif - return 0; } +#else +int main() { } +#endif --Zlo2GrDIozf6aQFY--