On Tue, Jan 30, 2024 at 04:34:30PM +0000, Jonathan Wakely wrote: > > --- /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 }
Why dg-timeout? I don't see it used in any of the libstdc++ tests and I don't see anything expensive on this test. Just leave it out. > > +// { dg-additional-options "-mlong-double-80" { target x86_64-*-* } } > > +// { dg-do run { target { ia32 || x86_64-*-* } } } Also, the target selectors above look wrong. Generally, one should use { i?86-*-* x86_64-*-* } because e.g. on some OSes like Solaris, one builds i?86 compiler which supports -m32 or -m64 options. I wouldn't add the -mlong-double-80 at all, it is an ABI changing option, if you rely in the test on some 80-bit long double properties (but on which ones? Sure, if -mlong-double-64 or -mlong-double-128 is in effect, the type won't have any padding, but __builtin_clear_padding should reflect that), it is better to guard the part of the test with some checks, whether std::numeric_limits<long double>::digits == 64, or preprocessor __LDBL_MANT_DIG__ == 64. > > +// { dg-add-options libatomic_16b } > > Why not just use -latomic unconditionally here? > > You're adding a new libatomic_16b option that expands to -latomic for > x86 and nothing otherwise, and then using it in a test that only runs > for x86. So it's just adding -latomic to all targets that run the > test, right? > > Also, this patch needs either a copyright assignment or a DCO sign-off: > https://gcc.gnu.org/contribute.html#legal And if somebody would need to commit it for you, also patch in non-garbled state (e.g. sent as an attachment, or even compressed attachment if using really bad MUAs). Jakub