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

[• Quuxplusone]

Summary

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.

[• Quuxplusone]

Rebase onto D110042. Enable the unit test for constinit-friendliness.

[• Quuxplusone]

Poke buildbot after landing D110042.

[ldionne]

On the one hand, the platform semaphore is going to work better because it uses the OS primitives, so that will respect whatever implementation is preferred on the OS. On the other hand, I find it surprising that the constructor is marked as constexpr if that can't be implemented using the most common implementation, i.e. pthread's sem_init. I'd like to see what @__simt__ or @wash have to say about this.

[__simt__]

Hmm. I lost track of how/when this constructor got defined as constexpr. It doesn't mean that it's unimplementable with platform semaphores, but to do so would make the implementation a lot more complicated, slower, and the object even larger than it already is.

I note that the MSVC STL also implemented semaphores exclusively on atomics, like this.

I'm personally pretty comfortable with this change.

[ldionne]

@Quuxplusone Please add a release note mentioning that we broke the ABI of counting_semaphore and ship it. Once you do, let me know and I'll cherry-pick to the release/13 branch.

[• Quuxplusone]

Add a release note. @ldionne, is this what you had in mind? I'm not clear on the procedure for release-noting things in general, let alone something that we're changing (I presume) between 13.0 and 13.1 — should that even get a 14.x release note? Anyway, please take a look and let me know before I'll land this in main. (Or just land this yourself if it looks good.)

[• Quuxplusone]

Rebased on main.
@ldionne ping! do you want a release note in 14.x or just when you put it in 13.x? is this a good release note?

[ldionne]

In D110110#3096959, @Quuxplusone wrote:

Rebased on main.
@ldionne ping! do you want a release note in 14.x or just when you put it in 13.x? is this a good release note?

I'll add the release note to 13.x when I cherry-pick it manually.

libcxx/docs/ReleaseNotes.rst

89 ↗

(On Diff #383388)

[ldionne]

Heads up @tstellar I'd like to cherry-pick this change to 13.x to minimize the impact of the ABI break (the sooner we release it, the less time folks have to ship code that would get bitten by the change).

[• Quuxplusone]

Copy UNSUPPORTED: linux && 32bits-on-64bits annotations from the other semaphore tests to deal with the failures in https://buildkite.com/llvm-project/libcxx-ci/builds/6278 . No idea why these are newly failing after this PR (as opposed to having already been failing when Louis annotated literally all the other semaphore tests).

If this passes CI, I'll go ahead and land it.

libcxx/docs/ReleaseNotes.rst

89 ↗

(On Diff #383388)

IIUC, you want me to eliminate the diff in ReleaseNotes.rst from this PR, and then you'll add it manually when you cherry-pick. The diff you'll add manually will be (mildly edited-yet-again by me):

- 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>``, so it is an ABI break. This change is needed to
  conform to the Standard, which requires ``counted_semaphore``'s constructor
  to be constexpr.

[ldionne]

LGTM. Yes, indeed, I will add that release note to LLVM 13 myself.

libcxx/include/__threading_support

 #ifndef _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER
 #pragma GCC system_header
 #endif
 
 #if defined(_LIBCPP_HAS_THREAD_API_EXTERNAL)
 # 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
 
 #if defined(_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL) || \
     defined(_LIBCPP_BUILDING_THREAD_LIBRARY_EXTERNAL) || \
     defined(_LIBCPP_HAS_THREAD_API_WIN32)
 #define _LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_FUNC_VIS

 #define _LIBCPP_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER
 
 typedef pthread_mutex_t __libcpp_recursive_mutex_t;
 
 // Condition Variable
 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
 
 // Thread id
 #if defined(__MVS__)
   typedef unsigned long long __libcpp_thread_id;
 #else

 #else
 # error Unsupported architecture
 #endif
 
 // Condition Variable
 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
 
 // Thread ID
 typedef long __libcpp_thread_id;
 
 // Thread

 
 _LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_NO_THREAD_SAFETY_ANALYSIS
 int __libcpp_condvar_timedwait(__libcpp_condvar_t *__cv, __libcpp_mutex_t *__m,
                                __libcpp_timespec_t *__ts);
 
 _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,
                           void (*init_routine)());
 
 // Thread id
 _LIBCPP_THREAD_ABI_VISIBILITY
 bool __libcpp_thread_id_equal(__libcpp_thread_id t1, __libcpp_thread_id t2);
int __libcpp_condvar_timedwait(__libcpp_condvar_t *__cv, __libcpp_mutex_t *__m,
   return pthread_cond_timedwait(__cv, __m, __ts);
 }
 
 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)()) {
   return pthread_once(flag, init_routine);
 }
 
 // Thread id
 // Returns non-zero if the thread ids are equal, otherwise 0

libcxx/include/semaphore

 #include <__undef_macros>
 
 #if _LIBCPP_STD_VER >= 14
 
 _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.
 
 */
 
 class __atomic_semaphore_base
 {
     __atomic_base<ptrdiff_t> __a;
bool try_acquire_for(chrono::duration<Rep, Period> const& __rel_time)
                 if(__a.compare_exchange_strong(__old, __old - 1, memory_order_acquire, memory_order_relaxed))
                     return true;
             }
         };
         return __libcpp_thread_poll_with_backoff(__test_fn, __libcpp_timed_backoff_policy(), __rel_time);
     }
 };
 
-#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 {
         return __least_max_value;
     }
 
     _LIBCPP_INLINE_VISIBILITY
     constexpr explicit counting_semaphore(ptrdiff_t __count) : __semaphore(__count) { }

libcxx/src/support/win32/thread_win32.cpp

 static_assert(alignof(__libcpp_thread_id) == alignof(DWORD), "");
 
 static_assert(sizeof(__libcpp_thread_t) == sizeof(HANDLE), "");
 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)
 {
   InitializeCriticalSection((LPCRITICAL_SECTION)__m);
   return 0;
 }
 
 int __libcpp_recursive_mutex_lock(__libcpp_recursive_mutex_t *__m)

 
 int __libcpp_tls_set(__libcpp_tls_key __key, void *__p)
 {
   if (!FlsSetValue(__key, __p))
     return GetLastError();
   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

libcxx/test/std/thread/thread.semaphore/acquire.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: libcpp-has-no-threads
 // UNSUPPORTED: c++03, c++11
 
 // This test requires the dylib support introduced in D68480, which shipped in
 // 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>
 #include <thread>
 
 #include "make_test_thread.h"
 #include "test_macros.h"
 

libcxx/test/std/thread/thread.semaphore/ctor.compile.pass.cpp

 #include "test_macros.h"
 
 static_assert(!std::is_default_constructible<std::binary_semaphore>::value, "");
 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

libcxx/test/std/thread/thread.semaphore/release.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: libcpp-has-no-threads
 // UNSUPPORTED: c++03, c++11
 
 // This test requires the dylib support introduced in D68480, which shipped in
 // 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>
 #include <thread>
 
 #include "make_test_thread.h"
 #include "test_macros.h"