On Tue, 16 Jan 2024 at 16:17, xndcn <[email protected]> wrote:
>
> Sorry about the mangled content...
> So I add a new add-options for libatomic_16b:
>
> ---
> 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.
> ---
> libstdc++-v3/include/bits/atomic_base.h | 7 ++-
> .../atomic_float/compare_exchange_padding.cc | 50 +++++++++++++++++++
> libstdc++-v3/testsuite/lib/dg-options.exp | 22 ++++++++
> 3 files changed, 78 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..104ddfdbe 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 __cplusplus >= 201402L && __has_builtin(__builtin_clear_padding)
> + if _GLIBCXX17_CONSTEXPR (__atomic_impl::__maybe_has_padding<_Fp>())
This code is not compiled for C++11 and C++14, so this should just use
constexpr not _GLIBCXX17_CONSTEXPR.
Apart from that I think this patch looks OK.
> + __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..d538b3d55
> --- /dev/null
> +++
> b/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.cc
> @@ -0,0 +1,50 @@
> +// { dg-do run { target c++20 } }
> +// { dg-options "-O0" }
> +// { dg-timeout 10 }
> +// { dg-additional-options "-mlong-double-80" { target x86_64-*-* } }
> +// { dg-do run { target { ia32 || x86_64-*-* } } }
> +// { dg-add-options libatomic_16b }
> +
> +#include <atomic>
> +#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()
> +{
> + 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..25da20d58 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
>
>
> Xi Ruoyao <[email protected]> 于2024年1月16日周二 18:12写道：
> >
> > On Tue, 2024-01-16 at 17:53 +0800, xndcn wrote:
> > > Thanks, so I add a test: atomic_float/compare_exchange_padding.cc,
> > > which will fail due to timeout without the patch.
> >
> > Please resend in plain text instead of HTML. Sending in HTML causes the
> > patch mangled.
> >
> > And libstdc++ patches should CC [email protected], in addition to
> > [email protected].
> >
> > > ---
> > > libstdc++-v3/ChangeLog:
> > >
> > > * include/bits/atomic_base.h: add __builtin_clear_padding in
> > > __atomic_float constructor.
> > > * testsuite/lib/dg-options.exp: enable libatomic for IA32 and X86-64.
> > > * testsuite/29_atomics/atomic_float/compare_exchange_padding.cc: New
> > > test.
> > > ---
> > > libstdc++-v3/include/bits/atomic_base.h | 7 ++-
> > > .../atomic_float/compare_exchange_padding.cc | 50 +++++++++++++++++++
> > > libstdc++-v3/testsuite/lib/dg-options.exp | 1 +
> > > 3 files changed, 57 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 d3a2c4f3805..cbe3749e17f 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 __cplusplus >= 201402L && __has_builtin(__builtin_clear_padding)
> > > + if _GLIBCXX17_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 00000000000..9376ab22850
> > > --- /dev/null
> > > +++
> > > b/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.cc
> > > @@ -0,0 +1,50 @@
> > > +// { dg-do run { target c++20 } }
> > > +// { dg-options "-O0" }
> > > +// { dg-timeout 10 }
> > > +// { dg-additional-options "-mlong-double-80" { target x86_64-*-* } }
> > > +// { dg-do run { target { ia32 || x86_64-*-* } } }
> > > +// { dg-add-options libatomic }
> > > +
> > > +#include <atomic>
> > > +#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()
> > > +{
> > > + 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 bc387d17ed7..d9a19dadd7f 100644
> > > --- a/libstdc++-v3/testsuite/lib/dg-options.exp
> > > +++ b/libstdc++-v3/testsuite/lib/dg-options.exp
> > > @@ -337,6 +337,7 @@ proc add_options_for_libatomic { flags } {
> > > || ([istarget powerpc*-*-*] && [check_effective_target_ilp32])
> > > || [istarget riscv*-*-*]
> > > || ([istarget sparc*-*-linux-gnu] && [check_effective_target_ilp32])
> > > + || ([istarget i?86-*-*] || [istarget x86_64-*-*])
> >
> > This seems too overkill as "dg-add-options libatomic" is not intended to
> > handle 16-byte atomics. Maybe we can fork this to a new dg-add-options
> > like "add_options_for_libatomic_16b"?
> >
> > > } {
> > > global TOOL_OPTIONS
> > >
> > > --
> > > 2.25.1
> >
> > --
> > Xi Ruoyao <[email protected]>
> > School of Aerospace Science and Technology, Xidian University
>
