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?


commit 07241d79b6720d4f392d5a8ba6e9c21d2801c8c7
Author: Jonathan Wakely <jwak...@redhat.com>
Date:   Wed Apr 21 13:04:05 2021

    libstdc++: Streamline <semaphore> implementation [PR 100164]
    
    This adds the missing _M_try_acquire member function so that
    __platform_semaphore works.
    
    Also adjust the <semaphore> implementation so that __platform_semaphore
    is not defined unless we're going to use it, which avoids including
    <semaphore.h> (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 <bits/atomic_base.h>
-#if __cpp_lib_atomic_wait
-#include <bits/atomic_timed_wait.h>
-#include <ext/numeric_traits.h>
-#endif // __cpp_lib_atomic_wait
-
-#ifdef _GLIBCXX_HAVE_POSIX_SEMAPHORE
-# include <limits.h>
-# include <semaphore.h>
-#endif
-
 #include <chrono>
 #include <type_traits>
 
+// 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 <bits/atomic_base.h>
+#if __cpp_lib_atomic_wait && ! _GLIBCXX_USE_POSIX_SEMAPHORE
+# include <bits/atomic_timed_wait.h>
+# include <ext/numeric_traits.h>
+#elif _GLIBCXX_HAVE_POSIX_SEMAPHORE
+# include <limits.h>
+# include <semaphore.h>
+# 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<int>::__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 <bits/semaphore_base.h>
 
+#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 <semaphore>
-#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 <chrono>
 #include <thread>
 #include <atomic>
 #include <testsuite_hooks.h>
 
+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<int> 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<int> 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

Reply via email to