Re: [PATCH] libstdc++: Clear padding bits in atomic compare_exchange

[Jonathan Wakely <jwakely@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

On Wed, 29 Sept 2021 at 13:13, Jonathan Wakely wrote:
> We repeat this *a lot*. When I started work on this I defined a
> non-member function in the __atomic_impl namespace:

I've attached my incomplete patch from when I was working on this.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 2929 bytes --]

diff --cc libstdc++-v3/include/bits/atomic_base.h
index 71e1de078b5,35023ad30a8..00000000000
--- a/libstdc++-v3/include/bits/atomic_base.h
+++ b/libstdc++-v3/include/bits/atomic_base.h
@@@ -944,6 -952,28 +944,27 @@@ _GLIBCXX_BEGIN_NAMESPACE_VERSIO
      template<typename _Tp>
        using _Val = remove_volatile_t<_Tp>;
  
+     template<typename _Tp>
+       constexpr bool
+       __maybe_has_padding()
+       {
 -#if _GLIBCXX_HAVE_BUILTIN_HAS_UNIQ_OBJ_REP
 -	return !__has_unique_object_representations(_Tp);
++#if __has_builtin(__has_unique_object_representations)
++	return !__has_unique_object_representations(_Tp)
++	  && !is_floating_point<_Tp>::value;
+ #else
+ 	return true;
+ #endif
+       }
+ 
+     template<typename _Tp>
+       _GLIBCXX_ALWAYS_INLINE void
+       __clear_padding(_Tp& __val) noexcept
+       {
 -	auto* __ptr = std::__addressof(__val);
+ #if __has_builtin(__builtin_clear_padding)
 -	__builtin_clear_padding(__ptr);
++	__builtin_clear_padding(std::__addressof(__val));
+ #endif
 -	return __ptr;
+       }
+ 
      // As above, but for difference_type arguments.
      template<typename _Tp>
        using _Diff = conditional_t<is_pointer_v<_Tp>, ptrdiff_t, _Val<_Tp>>;
@@@ -984,13 -1015,26 +1006,26 @@@
      template<typename _Tp>
        _GLIBCXX_ALWAYS_INLINE bool
        compare_exchange_weak(_Tp* __ptr, _Val<_Tp>& __expected,
- 			    _Val<_Tp> __desired, memory_order __success,
- 			    memory_order __failure) noexcept
+ 			    _Val<_Tp> __desired,
+ 			    memory_order __success, memory_order __failure,
+ 			    bool __weak = true) noexcept
        {
 +	__glibcxx_assert(__is_valid_cmpexch_failure_order(__failure));
- 
+ #if __has_builtin(__builtin_clear_padding)
+ 	if _GLIBCXX_CONSTEXPR17 (__maybe_has_padding<_Tp>())
+ 	  {
+ 	    _Val<_Tp> __expected0 = __expected;
+ 	    auto* __exp = __atomic_impl::__clear_padding(__expected0);
+ 	    auto* __des = __atomic_impl::__clear_padding(__desired);
+ 	    if (__atomic_compare_exchange(__ptr, __exp, __des, __weak,
+ 					  int(__success), int(__failure)))
+ 	      return true;
+ 	    __builtin_memcpy(std::__addressof(__expected), __exp, sizeof(_Tp));
+ 	    return false;
+ 	  }
 -	else
+ #endif
  	return __atomic_compare_exchange(__ptr, std::__addressof(__expected),
- 					 std::__addressof(__desired), true,
+ 					 std::__addressof(__desired), __weak,
  					 int(__success), int(__failure));
        }
  
@@@ -1000,11 -1044,8 +1035,9 @@@
  			      _Val<_Tp> __desired, memory_order __success,
  			      memory_order __failure) noexcept
        {
 +	__glibcxx_assert(__is_valid_cmpexch_failure_order(__failure));
- 
- 	return __atomic_compare_exchange(__ptr, std::__addressof(__expected),
- 					 std::__addressof(__desired), false,
- 					 int(__success), int(__failure));
+ 	return compare_exchange_weak(__ptr, __expected, __desired, __success,
+ 				     __failure, false);
        }
  
  #if __cpp_lib_atomic_wait