Re: [PATCH] libstdc++: atomic: Add missing clear_padding in __atomic_float constructor

[xndcn <xndchn@gmail.com>]

Thank you for your careful review!

> But we don't need a new one if it's going to be used in exactly one test and if the new option does the same thing for all targets that run the test.
Got it, thanks. Now add option "-latomic" directly, but it still rely
on the trick "[atomic_link_flags [get_multilibs]]"

> No, because the patch is supposed to prevent the infinite loop, and so there's no need to stop it looping after 10s. It won't loop at all.
Thanks, deleted.

> We only need to clear padding for long double, not float and double, right?
Yes, actually there is a check "if constexpr
(__atomic_impl::__maybe_has_padding<_Fp>())".
But "__atomic_impl::__clear_padding(_M_fp); " is indeed simply, so fixed here.

> Why can't we run this on all targets?
Got it, now target option deleted.

> There's no reason to use __builtin_memset here, just include <cstring> and use std::memcpy.
Thanks, fixed.

> It definitely does have padding, just say "long double has padding bits on x86"
Thanks, fixed.

So here comes the latest patch:
---
libstdc++-v3/ChangeLog:

 * include/bits/atomic_base.h: add __builtin_clear_padding in
__atomic_float constructor.
 * testsuite/29_atomics/atomic_float/compare_exchange_padding.cc: New test.

Signed-off-by: xndcn <xndchn@gmail.com>
---
 libstdc++-v3/include/bits/atomic_base.h       |  2 +-
 .../atomic_float/compare_exchange_padding.cc  | 53 +++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletion(-)
 create mode 100644
libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.cc

diff --git a/libstdc++-v3/include/bits/atomic_base.h
b/libstdc++-v3/include/bits/atomic_base.h
index f4ce0fa53..cdd6f07da 100644
--- a/libstdc++-v3/include/bits/atomic_base.h
+++ b/libstdc++-v3/include/bits/atomic_base.h
@@ -1283,7 +1283,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION

       constexpr
       __atomic_float(_Fp __t) : _M_fp(__t)
-      { }
+      { __atomic_impl::__clear_padding(_M_fp); }

       __atomic_float(const __atomic_float&) = delete;
       __atomic_float& operator=(const __atomic_float&) = delete;
diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.cc
b/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.cc
new file mode 100644
index 000000000..eeace251c
--- /dev/null
+++ b/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.cc
@@ -0,0 +1,53 @@
+// { dg-do run { target c++20 } }
+// { dg-options "-O0" }
+// { dg-additional-options "[atomic_link_flags [get_multilibs]] -latomic" }
+
+#include <atomic>
+#include <cstring>
+#include <limits>
+#include <testsuite_hooks.h>
+
+template<typename T>
+void __attribute__((noinline,noipa))
+fill_padding(T& f)
+{
+  T mask;
+  std::memset(&mask, 0xff, sizeof(T));
+  __builtin_clear_padding(&mask);
+  unsigned char* ptr_f = (unsigned char*)&f;
+  unsigned char* ptr_mask = (unsigned char*)&mask;
+  for (int i = 0; i < sizeof(T); i++)
+  {
+    if (ptr_mask[i] == 0x00)
+    {
+      ptr_f[i] = 0xff;
+    }
+  }
+}
+
+void
+test01()
+{
+  // test for long double with padding (float80)
+  if constexpr (std::numeric_limits<long double>::digits == 64)
+  {
+    long double f = 0.5f; // long double has padding bits on x86
+    fill_padding(f);
+    std::atomic<long double> as{ f }; // padding cleared on constructor
+    long double t = 1.5;
+
+    as.fetch_add(t);
+    long double s = f + t;
+    t = as.load();
+    VERIFY(s == t); // padding ignored on float comparing
+    fill_padding(s);
+    VERIFY(as.compare_exchange_weak(s, f)); // padding cleared on cmpexchg
+    fill_padding(f);
+    VERIFY(as.compare_exchange_strong(f, t)); // padding cleared on cmpexchg
+  }
+}
+
+int main()
+{
+  test01();
+}
-- 
2.25.1