[PATCH] [libstdc++] Refactor/cleanup of atomic wait implementation

[Jonathan Wakely]

On 19/04/21 12:23 -0700, Thomas Rodgers wrote:
>   namespace __detail
>   {
>-    using __platform_wait_t = int;
>-
>-    constexpr auto __atomic_spin_count_1 = 16;
>-    constexpr auto __atomic_spin_count_2 = 12;
>-
>-    template<typename _Tp>
>-      inline constexpr bool __platform_wait_uses_type
> #ifdef _GLIBCXX_HAVE_LINUX_FUTEX
>-	= is_same_v<remove_cv_t<_Tp>, __platform_wait_t>;
>+    using __platform_wait_t = int;
> #else
>-	= false;
>+    using __platform_wait_t = uint64_t;
>+#endif
>+    static constexpr size_t __platform_wait_alignment
>+				      = alignof(__platform_wait_t);

The value of this constant can't be alignof(__platform_wait_t). As
discussed, a futex always needs 4-byte alignment, but on at least one
target that GCC supports, alignof(int) == 2.

>+  } // namespace __detail
>+
>+  template<typename _Tp>
>+    inline constexpr bool __platform_wait_uses_type
>+#ifdef _GLIBCXX_HAVE_PLATFORM_WAIT
>+      = is_scalar_v<_Tp>
>+	&& ((sizeof(_Tp) == sizeof(__detail::__platform_wait_t))
>+	&& (alignof(_Tp*) >= alignof(__detail::__platform_wait_t)));

Now that we have the __platform_wait_alignment it should be used here
(so that when we fix the constant, this gets fixed too).

>+#else
>+      = false;
> #endif
>
>+  namespace __detail
>+  {
> #ifdef _GLIBCXX_HAVE_LINUX_FUTEX
>+#define _GLIBCXX_HAVE_PLATFORM_WAIT 1
>     enum class __futex_wait_flags : int
>     {
> #ifdef _GLIBCXX_HAVE_LINUX_FUTEX_PRIVATE




>+
>+	  static __waiter_type&
>+	  _S_for(const void* __addr)
>+	  {
>+	    static_assert(sizeof(__waiter_type) == sizeof(__waiter_pool_base));
>+	    auto& res = __waiter_pool_base::_S_for(__addr);
>+	    return reinterpret_cast<__waiter_type&>(res);
>+	  }

Nit: this is still indented as if it were a function template.

>       : _M_a(__expected) { }
>@@ -73,8 +73,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>     _GLIBCXX_ALWAYS_INLINE void
>     wait() const noexcept
>     {
>-      auto const __old = __atomic_impl::load(&_M_a, memory_order::acquire);
>-      std::__atomic_wait(&_M_a, __old, [this] { return this->try_wait(); });
>+      auto const __pred = [this] { return this->try_wait(); };
>+      std::__atomic_wait_address(&_M_a, __pred);
>     }
>
>     _GLIBCXX_ALWAYS_INLINE void
>@@ -85,7 +85,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>     }
>
>   private:
>-    alignas(__alignof__(ptrdiff_t)) ptrdiff_t _M_a;
>+    alignas(__alignof__(__detail::__platform_wait_t)) __detail::__platform_wait_t _M_a;

This should use the new constant too.