From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x135.google.com (mail-lf1-x135.google.com [IPv6:2a00:1450:4864:20::135]) by sourceware.org (Postfix) with ESMTPS id 65FB53858D28; Tue, 16 Jan 2024 16:17:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 65FB53858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 65FB53858D28 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::135 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705421823; cv=none; b=xFqg1A0u0sZfLBP4ma7L8DQC0VezJlzyU0bkJmi0boXGQ3vVa/+TAVWKqawAswYVzckRdzGb9Q5qINHVUk/j2L+zX1dWYweVMoD8vFlWh5ddtNr5RKUj7y2ivACka/T/8MkcTg+WVnIJHFWYJutPDTB6GIORUCxpISL8kOtsw+g= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705421823; c=relaxed/simple; bh=ntbOwQQxQjWqbNxcHzhfJNU8KvyDy4L2g2Zd5lqqSL4=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=pdSZBGFMxXTeAesimROkrL6Pg+zb/MqM2fvRdr5KOxwyeRfhP8ucwKafJcCexlVWDcF+Rngq+7iYH5fnMqDKyhdY26FbN01+x0V7sb5ZvjyvNvfxzJwICNGE9gKPc2LornP2ezg/CsaEmc48IO4ndj7+tlIvTCUe7eT8w1SQWCA= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-lf1-x135.google.com with SMTP id 2adb3069b0e04-50e78f1f41fso10635804e87.2; Tue, 16 Jan 2024 08:17:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1705421820; x=1706026620; darn=gcc.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=/IITmIfrdyWv5bHS8mHVnG9wfAM8igAXSbV+98Ci0kg=; b=jhscwXcFpQTUgM0orzmLXNz64vB2VcN0TdWJmMPIooXCBekngeeCHQGw1qcVux3aLJ yBKoOli6FvQxBxZjmoGSfPnpka1qG5UZ8YPrWodc/LbHhykAuSpGOslh9yvAbUU2n/Mc bdqjuVWlstMNKxU2Ak8uxe9o7twOY2+G7t+WCHU/JQCl8gypSP4Y+klFQ7AYm2DMeMK1 cdMNv1UfpiA3XoSTR2ucmKKhiaH/vo4tCFhG7WTOZNkztQoova4+Z8KBZiCDcpuEDA/C AhcbzWLxy7emoEs69s63lp4llt186dPi1jT+xQ/8eVSWi3tcCimIQfHDJgTulyXOt2VF NArQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705421820; x=1706026620; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=/IITmIfrdyWv5bHS8mHVnG9wfAM8igAXSbV+98Ci0kg=; b=I0hrckJ49wFs5drYqHQ0JZRsXWy0lfEiWuYJepHko29bIuSe/R6I7WGL8jgwb3z2/5 pxpIFaViqp55wv0PfgIcyg2B+a5UboqnfOUyw5qROro5d8Lfg6JH95Qj6tsLLQwfP9lO VZyCFJKQHhEFCjUL3F7VdKBPr63x6wnSxlV1aEo3swt5rtpHYgjaCzl+8SY2uYfgCXj2 nWh1Zuvjo4c6q4Sh49pdWeuuZQ5qifcAAqTtRoiWPt9Ldx4XKC+t/xe1WNCGxy9tDuC1 jgR7/PxMI9v/50cjSwGRv7GwzqILTWl7omuVvYpIW4DzeVg/LrJD2CDcXpfG9l+ejrlD 0mEw== X-Gm-Message-State: AOJu0YzoHxg5ziYKq5zTATY/nTtmGimUNkkRs9+sCSfNkemNRMY/RSPW Q+PFyK2G4yRsskpaQWbRHqPmfB6WE/SdkfzXv+ka2FxFIg0= X-Google-Smtp-Source: AGHT+IHViPJVBdEk/O4n646TYwo41fRFhfeUcSmrTC0pyJRAvGPz2mCy2cDPYR7ru+aIRYF02HBDiYhJ6Zy3KIA8P6s= X-Received: by 2002:a05:6512:33c3:b0:50e:aa03:a50f with SMTP id d3-20020a05651233c300b0050eaa03a50fmr3972868lfg.29.1705421819535; Tue, 16 Jan 2024 08:16:59 -0800 (PST) MIME-Version: 1.0 References: <9f2bdd3a973d98199bb5b322baa575ab2fba8a58.camel@xry111.site> In-Reply-To: <9f2bdd3a973d98199bb5b322baa575ab2fba8a58.camel@xry111.site> From: xndcn Date: Wed, 17 Jan 2024 00:16:47 +0800 Message-ID: Subject: Re: [PATCH] libstdc++: atomic: Add missing clear_padding in __atomic_float constructor To: Xi Ruoyao Cc: "H.J. Lu" , GCC Patches , Jakub Jelinek , libstdc++@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-6.2 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 >=3D 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&) =3D delete; __atomic_float& operator=3D(const __atomic_float&) =3D delete; diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchang= e_padding.cc b/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_padding.c= c new file mode 100644 index 000000000..d538b3d55 --- /dev/null +++ b/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_paddi= ng.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 +#include + +template +void __attribute__((noinline,noipa)) +fill_padding(T& f) +{ + T mask; + __builtin_memset(&mask, 0xff, sizeof(T)); + __builtin_clear_padding(&mask); + unsigned char* ptr_f =3D (unsigned char*)&f; + unsigned char* ptr_mask =3D (unsigned char*)&mask; + for (int i =3D 0; i < sizeof(T); i++) + { + if (ptr_mask[i] =3D=3D 0x00) + { + ptr_f[i] =3D 0xff; + } + } +} + +void +test01() +{ + long double f =3D 0.5f; // long double may contains padding on X86 + fill_padding(f); + std::atomic as{ f }; // padding cleared on constructor + long double t =3D 1.5; + + as.fetch_add(t); + long double s =3D f + t; + t =3D as.load(); + VERIFY(s =3D=3D 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=3D1= " --=20 2.25.1 Xi Ruoyao =E4=BA=8E2024=E5=B9=B41=E6=9C=8816=E6=97=A5= =E5=91=A8=E4=BA=8C 18:12=E5=86=99=E9=81=93=EF=BC=9A > > 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 libstdc++@gcc.gnu.org, in addition to > gcc-patches@gcc.gnu.org. > > > --- > > 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 t= est. > > --- > > 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/comp= are_exchange_padding.cc > > > > diff --git a/libstdc++-v3/include/bits/atomic_base.h b/libstdc++-v3/inc= lude/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 >=3D 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&) =3D delete; > > __atomic_float& operator=3D(const __atomic_float&) =3D delete; > > diff --git a/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exc= hange_padding.cc b/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_e= xchange_padding.cc > > new file mode 100644 > > index 00000000000..9376ab22850 > > --- /dev/null > > +++ b/libstdc++-v3/testsuite/29_atomics/atomic_float/compare_exchange_p= adding.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 > > +#include > > + > > +template > > +void __attribute__((noinline,noipa)) > > +fill_padding(T& f) > > +{ > > + T mask; > > + __builtin_memset(&mask, 0xff, sizeof(T)); > > + __builtin_clear_padding(&mask); > > + unsigned char* ptr_f =3D (unsigned char*)&f; > > + unsigned char* ptr_mask =3D (unsigned char*)&mask; > > + for (int i =3D 0; i < sizeof(T); i++) > > + { > > + if (ptr_mask[i] =3D=3D 0x00) > > + { > > + ptr_f[i] =3D 0xff; > > + } > > + } > > +} > > + > > +void > > +test01() > > +{ > > + long double f =3D 0.5f; // long double may contains padding on X86 > > + fill_padding(f); > > + std::atomic as{ f }; // padding cleared on constructor > > + long double t =3D 1.5; > > + > > + as.fetch_add(t); > > + long double s =3D f + t; > > + t =3D as.load(); > > + VERIFY(s =3D=3D t); // padding ignored on float comparing > > + fill_padding(s); > > + VERIFY(as.compare_exchange_weak(s, f)); // padding cleared on cmpexc= hg > > + fill_padding(f); > > + VERIFY(as.compare_exchange_strong(f, t)); // padding cleared on cmpe= xchg > > +} > > + > > +int main() > > +{ > > + test01(); > > +} > > diff --git a/libstdc++-v3/testsuite/lib/dg-options.exp b/libstdc++-v3/t= estsuite/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 > School of Aerospace Science and Technology, Xidian University