[libc++] Remove non-atomic "platform" semaphore implementations.

Arthur O'Dwyer · ldionne · commit 00f64ccb49d9 · 2021-11-05T09:58:10.000-04:00
These can't be made constexpr-constructible (constinit'able), so they aren't C++20-conforming. Also, the platform versions are going to be bigger than the atomic/futex version, so we'd have the awkward situation that `semaphore<42>` could be bigger than `semaphore<43>`, and that's just silly. Differential Revision: https://reviews.llvm.org/D110110 (cherry picked from commit d0eaf75) CHERRY-PICK NOTE BY @ldionne: I added a release note mentioning the ABI break.
diff --git a/libcxx/docs/ReleaseNotes.rst b/libcxx/docs/ReleaseNotes.rst
@@ -61,8 +61,8 @@ New Features
 - The documentation conversion from html to restructured text has been
   completed.
 
-API Changes
------------
+API and ABI Changes
+-------------------
 
 - There has been several changes in the tuple constructors provided by libc++.
   Those changes were made as part of an effort to regularize libc++'s tuple
@@ -95,3 +95,10 @@ API Changes
 
 - The ``std::result_of`` and ``std::is_literal_type`` type traits have been removed in
   C++20 mode.
+
+- The C++20 type ``std::counted_semaphore<N>`` is now based on ``std::atomic``
+  on all platforms, and does not use "native" semaphores such as pthreads
+  ``sem_t`` even on platforms that would support them. This changes the layout
+  of ``counted_semaphore<N>`` notably on Linux, so it is an ABI break on that
+  platform. This change is needed to conform to the Standard, which requires
+  ``counted_semaphore``'s constructor to be constexpr.
diff --git a/libcxx/include/__threading_support b/libcxx/include/__threading_support
@@ -29,16 +29,9 @@
 # include <__external_threading>
 #elif !defined(_LIBCPP_HAS_NO_THREADS)
 
-#if defined(__APPLE__) || defined(__MVS__)
-# define _LIBCPP_NO_NATIVE_SEMAPHORES
-#endif
-
 #if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
 # include <pthread.h>
 # include <sched.h>
-# ifndef _LIBCPP_NO_NATIVE_SEMAPHORES
-#   include <semaphore.h>
-# endif
 #elif defined(_LIBCPP_HAS_THREAD_API_C11)
 # include <threads.h>
 #endif
@@ -78,12 +71,6 @@ typedef pthread_mutex_t __libcpp_recursive_mutex_t;
 typedef pthread_cond_t __libcpp_condvar_t;
 #define _LIBCPP_CONDVAR_INITIALIZER PTHREAD_COND_INITIALIZER
 
-#ifndef _LIBCPP_NO_NATIVE_SEMAPHORES
-// Semaphore
-typedef sem_t __libcpp_semaphore_t;
-# define _LIBCPP_SEMAPHORE_MAX SEM_VALUE_MAX
-#endif
-
 // Execute once
 typedef pthread_once_t __libcpp_exec_once_flag;
 #define _LIBCPP_EXEC_ONCE_INITIALIZER PTHREAD_ONCE_INIT
@@ -149,12 +136,6 @@ typedef void* __libcpp_recursive_mutex_t[5];
 typedef void* __libcpp_condvar_t;
 #define _LIBCPP_CONDVAR_INITIALIZER 0
 
-// Semaphore
-typedef void* __libcpp_semaphore_t;
-#if defined(_LIBCPP_HAS_THREAD_API_WIN32)
-# define _LIBCPP_SEMAPHORE_MAX (::std::numeric_limits<long>::max())
-#endif
-
 // Execute Once
 typedef void* __libcpp_exec_once_flag;
 #define _LIBCPP_EXEC_ONCE_INITIALIZER 0
@@ -219,26 +200,6 @@ int __libcpp_condvar_timedwait(__libcpp_condvar_t *__cv, __libcpp_mutex_t *__m,
 _LIBCPP_THREAD_ABI_VISIBILITY
 int __libcpp_condvar_destroy(__libcpp_condvar_t* __cv);
 
-#ifndef _LIBCPP_NO_NATIVE_SEMAPHORES
-
-// Semaphore
-_LIBCPP_THREAD_ABI_VISIBILITY
-bool __libcpp_semaphore_init(__libcpp_semaphore_t* __sem, int __init);
-
-_LIBCPP_THREAD_ABI_VISIBILITY
-bool __libcpp_semaphore_destroy(__libcpp_semaphore_t* __sem);
-
-_LIBCPP_THREAD_ABI_VISIBILITY
-bool __libcpp_semaphore_post(__libcpp_semaphore_t* __sem);
-
-_LIBCPP_THREAD_ABI_VISIBILITY
-bool __libcpp_semaphore_wait(__libcpp_semaphore_t* __sem);
-
-_LIBCPP_THREAD_ABI_VISIBILITY
-bool __libcpp_semaphore_wait_timed(__libcpp_semaphore_t* __sem, chrono::nanoseconds const& __ns);
-
-#endif // _LIBCPP_NO_NATIVE_SEMAPHORES
-
 // Execute once
 _LIBCPP_THREAD_ABI_VISIBILITY
 int __libcpp_execute_once(__libcpp_exec_once_flag *flag,
@@ -452,38 +413,6 @@ int __libcpp_condvar_destroy(__libcpp_condvar_t *__cv)
   return pthread_cond_destroy(__cv);
 }
 
-#ifndef _LIBCPP_NO_NATIVE_SEMAPHORES
-
-// Semaphore
-bool __libcpp_semaphore_init(__libcpp_semaphore_t* __sem, int __init)
-{
-    return sem_init(__sem, 0, __init) == 0;
-}
-
-bool __libcpp_semaphore_destroy(__libcpp_semaphore_t* __sem)
-{
-    return sem_destroy(__sem) == 0;
-}
-
-bool __libcpp_semaphore_post(__libcpp_semaphore_t* __sem)
-{
-    return sem_post(__sem) == 0;
-}
-
-bool __libcpp_semaphore_wait(__libcpp_semaphore_t* __sem)
-{
-    return sem_wait(__sem) == 0;
-}
-
-bool __libcpp_semaphore_wait_timed(__libcpp_semaphore_t* __sem, chrono::nanoseconds const& __ns)
-{
-    auto const __abs_time = chrono::system_clock::now().time_since_epoch() + __ns;
-    __libcpp_timespec_t __ts = __thread_detail::__convert_to_timespec(__abs_time);
-    return sem_timedwait(__sem, &__ts) == 0;
-}
-
-#endif //_LIBCPP_NO_NATIVE_SEMAPHORES
-
 // Execute once
 int __libcpp_execute_once(__libcpp_exec_once_flag *flag,
                           void (*init_routine)()) {
diff --git a/libcxx/include/semaphore b/libcxx/include/semaphore
@@ -67,10 +67,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 
 /*
 
-__atomic_semaphore_base is the general-case implementation, to be used for
-user-requested least-max values that exceed the OS implementation support
-(incl. when the OS has no support of its own) and for binary semaphores.
-
+__atomic_semaphore_base is the general-case implementation.
 It is a typical Dijkstra semaphore algorithm over atomics, wait and notify
 functions. It avoids contention against users' own use of those facilities.
 
@@ -121,68 +118,12 @@ public:
     }
 };
 
-#ifndef _LIBCPP_NO_NATIVE_SEMAPHORES
-
-/*
-
-__platform_semaphore_base a simple wrapper for the OS semaphore type. That
-is, every call is routed to the OS in the most direct manner possible.
-
-*/
-
-class __platform_semaphore_base
-{
-    __libcpp_semaphore_t __semaphore;
-
-public:
-    _LIBCPP_INLINE_VISIBILITY
-    explicit __platform_semaphore_base(ptrdiff_t __count) :
-        __semaphore()
-    {
-        __libcpp_semaphore_init(&__semaphore, __count);
-    }
-    _LIBCPP_INLINE_VISIBILITY
-    ~__platform_semaphore_base() {
-        __libcpp_semaphore_destroy(&__semaphore);
-    }
-    _LIBCPP_INLINE_VISIBILITY
-    void release(ptrdiff_t __update)
-    {
-        for(; __update; --__update)
-            __libcpp_semaphore_post(&__semaphore);
-    }
-    _LIBCPP_INLINE_VISIBILITY
-    void acquire()
-    {
-        __libcpp_semaphore_wait(&__semaphore);
-    }
-    _LIBCPP_INLINE_VISIBILITY
-    bool try_acquire_for(chrono::nanoseconds __rel_time)
-    {
-        return __libcpp_semaphore_wait_timed(&__semaphore, __rel_time);
-    }
-};
-
-template<ptrdiff_t __least_max_value>
-using __semaphore_base =
-    typename conditional<(__least_max_value > 1 && __least_max_value <= _LIBCPP_SEMAPHORE_MAX),
-                         __platform_semaphore_base,
-                         __atomic_semaphore_base>::type;
-
-#else
-
-template<ptrdiff_t __least_max_value>
-using __semaphore_base =
-    __atomic_semaphore_base;
-
 #define _LIBCPP_SEMAPHORE_MAX (numeric_limits<ptrdiff_t>::max())
 
-#endif //_LIBCPP_NO_NATIVE_SEMAPHORES
-
 template<ptrdiff_t __least_max_value = _LIBCPP_SEMAPHORE_MAX>
 class counting_semaphore
 {
-    __semaphore_base<__least_max_value> __semaphore;
+    __atomic_semaphore_base __semaphore;
 
 public:
     static constexpr ptrdiff_t max() noexcept {
diff --git a/libcxx/src/support/win32/thread_win32.cpp b/libcxx/src/support/win32/thread_win32.cpp
@@ -39,9 +39,6 @@ static_assert(alignof(__libcpp_thread_t) == alignof(HANDLE), "");
 static_assert(sizeof(__libcpp_tls_key) == sizeof(DWORD), "");
 static_assert(alignof(__libcpp_tls_key) == alignof(DWORD), "");
 
-static_assert(sizeof(__libcpp_semaphore_t) == sizeof(HANDLE), "");
-static_assert(alignof(__libcpp_semaphore_t) == alignof(HANDLE), "");
-
 // Mutex
 int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t *__m)
 {
@@ -275,37 +272,4 @@ int __libcpp_tls_set(__libcpp_tls_key __key, void *__p)
   return 0;
 }
 
-// Semaphores
-bool __libcpp_semaphore_init(__libcpp_semaphore_t* __sem, int __init)
-{
-  *(PHANDLE)__sem = CreateSemaphoreEx(nullptr, __init, _LIBCPP_SEMAPHORE_MAX,
-                                      nullptr, 0, SEMAPHORE_ALL_ACCESS);
-  return *__sem != nullptr;
-}
-
-bool __libcpp_semaphore_destroy(__libcpp_semaphore_t* __sem)
-{
-  CloseHandle(*(PHANDLE)__sem);
-  return true;
-}
-
-bool __libcpp_semaphore_post(__libcpp_semaphore_t* __sem)
-{
-  return ReleaseSemaphore(*(PHANDLE)__sem, 1, nullptr);
-}
-
-bool __libcpp_semaphore_wait(__libcpp_semaphore_t* __sem)
-{
-  return WaitForSingleObjectEx(*(PHANDLE)__sem, INFINITE, false) ==
-         WAIT_OBJECT_0;
-}
-
-bool __libcpp_semaphore_wait_timed(__libcpp_semaphore_t* __sem,
-                                   chrono::nanoseconds const& __ns)
-{
-  chrono::milliseconds __ms = chrono::ceil<chrono::milliseconds>(__ns);
-  return WaitForSingleObjectEx(*(PHANDLE)__sem, __ms.count(), false) ==
-         WAIT_OBJECT_0;
-}
-
 _LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/test/std/thread/thread.semaphore/acquire.pass.cpp b/libcxx/test/std/thread/thread.semaphore/acquire.pass.cpp
@@ -13,6 +13,9 @@
 // macOS 11.0.
 // XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11|12|13|14|15}}
 
+// TODO(ldionne): This test fails on Ubuntu Focal on our CI nodes (and only there), in 32 bit mode.
+// UNSUPPORTED: linux && 32bits-on-64bits
+
 // <semaphore>
 
 #include <semaphore>
diff --git a/libcxx/test/std/thread/thread.semaphore/ctor.compile.pass.cpp b/libcxx/test/std/thread/thread.semaphore/ctor.compile.pass.cpp
@@ -24,7 +24,8 @@ static_assert(!std::is_default_constructible<std::counting_semaphore<>>::value,
 static_assert(!std::is_convertible<int, std::binary_semaphore>::value, "");
 static_assert(!std::is_convertible<int, std::counting_semaphore<>>::value, "");
 
-#if 0 // TODO FIXME: the ctor should be constexpr when TEST_STD_VER > 17
+#if TEST_STD_VER > 17
+// Test constexpr-constructibility. (But not destructibility.)
 constinit std::binary_semaphore bs(1);
 constinit std::counting_semaphore cs(1);
 #endif
diff --git a/libcxx/test/std/thread/thread.semaphore/release.pass.cpp b/libcxx/test/std/thread/thread.semaphore/release.pass.cpp
@@ -13,6 +13,9 @@
 // macOS 11.0.
 // XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11|12|13|14|15}}
 
+// TODO(ldionne): This test fails on Ubuntu Focal on our CI nodes (and only there), in 32 bit mode.
+// UNSUPPORTED: linux && 32bits-on-64bits
+
 // <semaphore>
 
 #include <semaphore>
