Thanks! > Why not just use -latomic unconditionally here? testsuites of libstdc++ seems to have some tricks to find and link libatomic.a by using "dg-add-options libatomic", which do nothing for x86 target. In previous email, we decide not to pollute the current option, so we add a new libatomic_16b option here.
> Why dg-timeout? without the patch, "as.fetch_add(t);" will result in an infinite loop, so I add timeout to help it escape. Should I keep it or not? > Also, the target selectors above look wrong. Thanks, fixed with checking "std::numeric_limits<long double>::digits == 64". > This code is not compiled for C++11 and C++14, so this should just use > constexpr not _GLIBCXX17_CONSTEXPR. Thanks, fixed with "constexpr". So here is the fixed patch, please review it again, thanks: --- libstdc++-v3/ChangeLog: * include/bits/atomic_base.h: add __builtin_clear_padding in __atomic_float constructor. * testsuite/lib/dg-options.exp: add new add-options for libatomic_16b. * testsuite/29_atomics/atomic_float/compare_exchange_padding.cc: New test. Signed-off-by: xndcn <[email protected]> --- libstdc++-v3/include/bits/atomic_base.h | 7 ++- .../atomic_float/compare_exchange_padding.cc | 54 +++++++++++++++++++ libstdc++-v3/testsuite/lib/dg-options.exp | 22 ++++++++ 3 files changed, 82 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..90e3f0e4b 100644 --- a/libstdc++-v3/include/bits/atomic_base.h +++ b/libstdc++-v3/include/bits/atomic_base.h @@ -1283,7 +1283,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr __atomic_float(_Fp __t) : _M_fp(__t) - { } + { +#if __has_builtin(__builtin_clear_padding) + if constexpr (__atomic_impl::__maybe_has_padding<_Fp>()) + __builtin_clear_padding(std::__addressof(_M_fp)); +#endif + } __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..e6e17e4b5 --- /dev/null +++ b/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.cc @@ -0,0 +1,54 @@ +// { dg-do run { target c++20 } } +// { dg-options "-O0" } +// { dg-timeout 10 } +// { dg-do run { target { i?86-*-* x86_64-*-* } } } +// { dg-add-options libatomic_16b } + +#include <atomic> +#include <limits> +#include <testsuite_hooks.h> + +template<typename T> +void __attribute__((noinline,noipa)) +fill_padding(T& f) +{ + T mask; + __builtin_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 may contains padding 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(); +} diff --git a/libstdc++-v3/testsuite/lib/dg-options.exp b/libstdc++-v3/testsuite/lib/dg-options.exp index 850442b6b..75b27a136 100644 --- a/libstdc++-v3/testsuite/lib/dg-options.exp +++ b/libstdc++-v3/testsuite/lib/dg-options.exp @@ -356,6 +356,28 @@ proc add_options_for_libatomic { flags } { return $flags } +# Add option to link to libatomic, if required for atomics on 16-byte (128-bit) +proc add_options_for_libatomic_16b { flags } { + if { ([istarget i?86-*-*] || [istarget x86_64-*-*]) + } { + global TOOL_OPTIONS + + set link_flags "" + if ![is_remote host] { + if [info exists TOOL_OPTIONS] { + set link_flags "[atomic_link_flags [get_multilibs ${TOOL_OPTIONS}]]" + } else { + set link_flags "[atomic_link_flags [get_multilibs]]" + } + } + + append link_flags " -latomic " + + return "$flags $link_flags" + } + return $flags +} + # Add options to enable use of deprecated features. proc add_options_for_using-deprecated { flags } { return "$flags -U_GLIBCXX_USE_DEPRECATED -D_GLIBCXX_USE_DEPRECATED=1" -- 2.25.1
