[2/2] libstdc++: Pass __wait_args to internal API by const pointer

Message ID 20231116134736.1287539-2-jwakely@redhat.com
State Accepted
Headers
Series [1/2] libstdc++: Atomic wait/notify ABI stabilization |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Jonathan Wakely Nov. 16, 2023, 1:45 p.m. UTC
  From: Thomas Rodgers <trodgers@redhat.com>

Tested x86_64-linux, testing underway on powerpc-aix and sparc-solaris.

-- >8 --

This change splits the __wait_args data members to a new struct
__wait_args_base and then passes that type by const pointer to the low
level implementation functions.

libstdc++-v3/ChangeLog:

	* include/bits/atomic_timed_wait.h (__spin_until_impl): Accept
	__wait_args as const __wait_args_base*.
	(__wait_until_impl): Likewise.
	(__wait_until): Likewise.
	(__wait_for): Likewise.
	(__atomic_wait_address_until): Pass __wait_args by address.
	(__atomic_wait_address_for): Likewise.
	* include/bits/atomic_wait.h (__wait_args_base): New struct.
	(__wait_args): Derive from __wait_args_base.
	(__wait_args::__wait_args()): Adjust ctors to call call base ctor.
	(__wait_args::__wait_args(const __wait_args_base&)): New ctor.
	(__wait_args::operator|=): New method.
	(__wait_args::_S_flags_for): Change return type to
	__wait_flags.
	(__spin_impl): Accept __wait_args as const __wait_args_base*.
	(__wait_impl): Likewise.
	(__notify_impl): Likewise.
	(__atomic_wait_address): Pass __wait_args by address.
	(__atomic_wait_address_v): Likewise.
	(__atomic_notify_address): Likewise.
---
 libstdc++-v3/include/bits/atomic_timed_wait.h | 35 +++++++----
 libstdc++-v3/include/bits/atomic_wait.h       | 62 +++++++++++++------
 2 files changed, 64 insertions(+), 33 deletions(-)
  

Patch

diff --git a/libstdc++-v3/include/bits/atomic_timed_wait.h b/libstdc++-v3/include/bits/atomic_timed_wait.h
index 90427ef8b42..6f6e8758ee2 100644
--- a/libstdc++-v3/include/bits/atomic_timed_wait.h
+++ b/libstdc++-v3/include/bits/atomic_timed_wait.h
@@ -133,9 +133,11 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif // _GLIBCXX_HAS_GTHREADS
 
     inline __wait_result_type
-    __spin_until_impl(const __platform_wait_t* __addr, __wait_args __args,
+    __spin_until_impl(const __platform_wait_t* __addr,
+		      const __wait_args_base* __a,
 		      const __wait_clock_t::time_point& __deadline)
     {
+      __wait_args __args{ *__a };
       auto __t0 = __wait_clock_t::now();
       using namespace literals::chrono_literals;
 
@@ -161,7 +163,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	}
 	else
 	{
-	  auto __res = __detail::__spin_impl(__addr, __args);
+	  auto __res = __detail::__spin_impl(__addr, __a);
 	  if (__res.first)
 	    return __res;
 	}
@@ -174,9 +176,11 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     }
 
     inline __wait_result_type
-    __wait_until_impl(const __platform_wait_t* __addr, __wait_args __args,
+    __wait_until_impl(const __platform_wait_t* __addr,
+		      const __wait_args_base* __a,
 		      const __wait_clock_t::time_point& __atime)
     {
+      __wait_args __args{ *__a };
 #ifdef _GLIBCXX_HAVE_PLATFORM_TIMED_WAIT
       __waiter_pool_impl* __pool = nullptr;
 #else
@@ -198,7 +202,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       if (__args & __wait_flags::__do_spin)
 	{
-	  auto __res = __detail::__spin_until_impl(__wait_addr, __args, __atime);
+	  auto __res = __detail::__spin_until_impl(__wait_addr, __a, __atime);
 	  if (__res.first)
 	    return __res;
 	  if (__args & __wait_flags::__spin_only)
@@ -244,7 +248,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     template<typename _Clock, typename _Dur>
       __wait_result_type
-      __wait_until(const __platform_wait_t* __addr, __wait_args __args,
+      __wait_until(const __platform_wait_t* __addr, const __wait_args* __args,
 		   const chrono::time_point<_Clock, _Dur>& __atime) noexcept
       {
 	if constexpr (is_same_v<__wait_clock_t, _Clock>)
@@ -267,15 +271,20 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     template<typename _Rep, typename _Period>
       __wait_result_type
-      __wait_for(const __platform_wait_t* __addr, __wait_args __args,
+      __wait_for(const __platform_wait_t* __addr, const __wait_args_base* __a,
 		 const chrono::duration<_Rep, _Period>& __rtime) noexcept
     {
+      __wait_args __args{ *__a };
       if (!__rtime.count())
-	// no rtime supplied, just spin a bit
-	return __detail::__wait_impl(__addr, __args | __wait_flags::__spin_only);
+	{
+	  // no rtime supplied, just spin a bit
+	  __args |= __wait_flags::__spin_only;
+	  return __detail::__wait_impl(__addr, &__args);
+	}
+
       auto const __reltime = chrono::ceil<__wait_clock_t::duration>(__rtime);
       auto const __atime = chrono::steady_clock::now() + __reltime;
-      return __detail::__wait_until(__addr, __args, __atime);
+      return __detail::__wait_until(__addr, &__args, __atime);
     }
   } // namespace __detail
 
@@ -295,7 +304,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
        _Tp __val = __vfn();
        while (!__pred(__val))
 	 {
-	   auto __res = __detail::__wait_until(__wait_addr, __args, __atime);
+	   auto __res = __detail::__wait_until(__wait_addr, &__args, __atime);
 	   if (!__res.first)
 	     // timed out
 	     return __res.first; // C++26 will also return last observed __val
@@ -313,7 +322,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 				  bool __bare_wait = false) noexcept
     {
       __detail::__wait_args __args{ __addr, __old, __order, __bare_wait };
-      auto __res = __detail::__wait_until(__addr, __args, __atime);
+      auto __res = __detail::__wait_until(__addr, &__args, __atime);
       return __res.first; // C++26 will also return last observed __val
     }
 
@@ -345,7 +354,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _Tp __val = __vfn();
       while (!__pred(__val))
 	{
-	  auto __res = __detail::__wait_for(__wait_addr, __args, __rtime);
+	  auto __res = __detail::__wait_for(__wait_addr, &__args, __rtime);
 	  if (!__res.first)
 	    // timed out
 	    return __res.first; // C++26 will also return last observed __val
@@ -363,7 +372,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 				bool __bare_wait = false) noexcept
   {
     __detail::__wait_args __args{ __addr, __old, __order, __bare_wait };
-    auto __res = __detail::__wait_for(__addr, __args, __rtime);
+    auto __res = __detail::__wait_for(__addr, &__args, __rtime);
     return __res.first; // C++26 will also return last observed __Val
   }
 
diff --git a/libstdc++-v3/include/bits/atomic_wait.h b/libstdc++-v3/include/bits/atomic_wait.h
index 63d132fc9a8..8abcdec3d50 100644
--- a/libstdc++-v3/include/bits/atomic_wait.h
+++ b/libstdc++-v3/include/bits/atomic_wait.h
@@ -212,23 +212,28 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
        __abi_version_mask = 0xffff0000,
     };
 
-    struct __wait_args
+    struct __wait_args_base
     {
-      __platform_wait_t _M_old;
-      int _M_order = __ATOMIC_ACQUIRE;
       __wait_flags _M_flags;
+      int _M_order = __ATOMIC_ACQUIRE;
+      __platform_wait_t _M_old = 0;
+    };
 
+    struct __wait_args : __wait_args_base
+    {
       template<typename _Tp>
 	explicit __wait_args(const _Tp* __addr,
 			     bool __bare_wait = false) noexcept
-	    : _M_flags{ _S_flags_for(__addr, __bare_wait) }
+	    : __wait_args_base{ _S_flags_for(__addr, __bare_wait) }
 	{ }
 
       __wait_args(const __platform_wait_t* __addr, __platform_wait_t __old,
 		  int __order, bool __bare_wait = false) noexcept
-	  : _M_old{ __old }
-	  , _M_order{ __order }
-	  , _M_flags{ _S_flags_for(__addr, __bare_wait) }
+	  : __wait_args_base{ _S_flags_for(__addr, __bare_wait), __order, __old }
+	{ }
+
+      explicit __wait_args(const __wait_args_base& __base)
+	  : __wait_args_base{ __base }
 	{ }
 
       __wait_args(const __wait_args&) noexcept = default;
@@ -254,6 +259,16 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	return __res;
       }
 
+      __wait_args&
+      operator|=(__wait_flags __flag) noexcept
+      {
+	using __t = underlying_type_t<__wait_flags>;
+	const auto __flags = static_cast<__t>(_M_flags)
+			     | static_cast<__t>(__flag);
+	_M_flags = __wait_flags{ __flags };
+	return *this;
+      }
+
     private:
       static int
       constexpr _S_default_flags() noexcept
@@ -264,7 +279,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       }
 
       template<typename _Tp>
-	static int
+	static __wait_flags
 	constexpr _S_flags_for(const _Tp*, bool __bare_wait) noexcept
 	{
 	  auto __res = _S_default_flags();
@@ -272,7 +287,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    __res |= static_cast<int>(__wait_flags::__track_contention);
 	  if constexpr (!__platform_wait_uses_type<_Tp>)
 	    __res |= static_cast<int>(__wait_flags::__proxy_wait);
-	  return __res;
+	  return static_cast<__wait_flags>(__res);
 	}
 
       template<typename _Tp>
@@ -287,13 +302,13 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     using __wait_result_type = pair<bool, __platform_wait_t>;
     inline __wait_result_type
-    __spin_impl(const __platform_wait_t* __addr, __wait_args __args)
+    __spin_impl(const __platform_wait_t* __addr, const __wait_args_base* __args)
     {
       __platform_wait_t __val;
       for (auto __i = 0; __i < __atomic_spin_count; ++__i)
 	{
-	  __atomic_load(__addr, &__val, __args._M_order);
-	  if (__val != __args._M_old)
+	  __atomic_load(__addr, &__val, __args->_M_order);
+	  if (__val != __args->_M_old)
 	    return make_pair(true, __val);
 	  if (__i < __atomic_spin_count_relax)
 	    __detail::__thread_relax();
@@ -304,8 +319,9 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     }
 
     inline __wait_result_type
-    __wait_impl(const __platform_wait_t* __addr, __wait_args __args)
+    __wait_impl(const __platform_wait_t* __addr, const __wait_args_base* __a)
     {
+      __wait_args __args{ *__a };
 #ifdef _GLIBCXX_HAVE_PLATFORM_WAIT
       __waiter_pool_impl* __pool = nullptr;
 #else
@@ -314,20 +330,25 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif
 
       __platform_wait_t* __wait_addr;
+      __platform_wait_t __old;
       if (__args & __wait_flags::__proxy_wait)
 	{
 #ifdef _GLIBCXX_HAVE_PLATFORM_WAIT
 	  __pool = &__waiter_pool_impl::_S_impl_for(__addr);
 #endif
 	  __wait_addr = &__pool->_M_ver;
-	  __atomic_load(__wait_addr, &__args._M_old, __args._M_order);
+	  __atomic_load(__wait_addr, &__old, __args._M_order);
 	}
       else
-	__wait_addr = const_cast<__platform_wait_t*>(__addr);
+	{
+	  __wait_addr = const_cast<__platform_wait_t*>(__addr);
+	  __old = __args._M_old;
+	}
+
 
       if (__args & __wait_flags::__do_spin)
 	{
-	  auto __res = __detail::__spin_impl(__wait_addr, __args);
+	  auto __res = __detail::__spin_impl(__wait_addr, __a);
 	  if (__res.first)
 	    return __res;
 	  if (__args & __wait_flags::__spin_only)
@@ -369,8 +390,9 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     inline void
     __notify_impl(const __platform_wait_t* __addr, [[maybe_unused]] bool __all,
-		  __wait_args __args)
+		  const __wait_args_base* __a)
     {
+      __wait_args __args{ __a };
 #ifdef _GLIBCXX_HAVE_PLATFORM_WAIT
       __waiter_pool_impl* __pool = nullptr;
 #else
@@ -421,7 +443,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _Tp __val = __vfn();
       while (!__pred(__val))
 	{
-	  __detail::__wait_impl(__wait_addr, __args);
+	  __detail::__wait_impl(__wait_addr, &__args);
 	  __val = __vfn();
 	}
       // C++26 will return __val
@@ -434,7 +456,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   {
     __detail::__wait_args __args{ __addr, __old, __order };
     // C++26 will not ignore the return value here
-    __detail::__wait_impl(__addr, __args);
+    __detail::__wait_impl(__addr, &__args);
   }
 
   template<typename _Tp, typename _ValFn>
@@ -455,7 +477,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       const auto __wait_addr =
 	  reinterpret_cast<const __detail::__platform_wait_t*>(__addr);
       __detail::__wait_args __args{ __addr, __bare_wait };
-      __detail::__notify_impl(__wait_addr, __all, __args);
+      __detail::__notify_impl(__wait_addr, __all, &__args);
     }
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace std