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

[Jonathan Wakely via Gcc-patches]

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.