On Mon, 11 Sept 2023 at 13:36, Christophe Lyon <christophe.l...@linaro.org> wrote: > > > > On Mon, 11 Sept 2023 at 12:59, Jonathan Wakely <jwak...@redhat.com> wrote: >> >> On Sun, 10 Sept 2023 at 20:31, Christophe Lyon >> <christophe.l...@linaro.org> wrote: >> > >> > Some targets like arm-eabi with newlib and default settings rely on >> > __sync_synchronize() to ensure synchronization. Newlib does not >> > implement it by default, to make users aware they have to take special >> > care. >> > >> > This makes a few tests fail to link. >> >> Does this mean those features are unusable on the target, or just that >> users need to provide their own __sync_synchronize to use them? > > > IIUC the user is expected to provide them. > Looks like we discussed this in the past :-) > In https://gcc.gnu.org/legacy-ml/gcc-patches/2016-10/msg01632.html, > see the pointer to Ramana's comment: > https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02751.html
Oh yes, thanks for the reminder! > > The default arch for arm-eabi is armv4t which is very old. > When running the testsuite with something more recent (either as default by > configuring GCC --with-arch=XXX or by forcing -march/-mcpu via dejagnu's > target-board), the compiler generates barrier instructions and there are no > such errors. Ah yes, that's fine then. > For instance, here is a log with the defaults: > https://git.linaro.org/toolchain/ci/base-artifacts/tcwg_gnu_embed_check_gcc/master-arm_eabi.git/tree/00-sumfiles?h=linaro-local/ci/tcwg_gnu_embed_check_gcc/master-arm_eabi > and a log when we target cortex-m0 which is still a very small cpu but has > barriers: > https://git.linaro.org/toolchain/ci/base-artifacts/tcwg_gnu_embed_check_gcc/master-thumb_m0_eabi.git/tree/00-sumfiles?h=linaro-local/ci/tcwg_gnu_embed_check_gcc/master-thumb_m0_eabi > > I somehow wanted to get rid of such errors with the default configuration.... Yep, that makes sense, and we'll still be testing them for newer arches on the target, so it's not completely disabling those parts of the testsuite. But I'm still curious why some of those tests need this change. I think the ones I noted below are probably failing for some other reasons. > >> >> > >> > This patch requires the missing thread-fence effective target in the >> > tests that need it, making them UNSUPPORTED instead of FAIL and >> > UNRESOLVED. >> >> Some of the modified tests should not be using >> __gnu_debug::_Safe_sequence_base::_M_detach_all(), because they don't >> use the Debug Mode. I don't know where those linker errors come from. >> For example, the 23_containers/span/*assert_neg.cc and >> 26_numerics/valarray/* tests shouldn't use debug iterators or atomics. >> Neither should 25_algorithms/sample/2.cc nor >> 26_numerics/bit/bit.pow.two/bit_ceil_neg.cc > > > Ouch! I had the feeling this patch wouldn't count as obvious :-) > > I confess I didn't analyze the linker map for every single test updated by > this patch.... > I can have a deeper look based on your comment below, excluding those that > look "OK" > >> >> The last three in the patch shouldn't use it either: >> >> > diff --git >> > a/libstdc++-v3/testsuite/experimental/net/timer/waitable/dest.cc >> > b/libstdc++-v3/testsuite/experimental/net/timer/waitable/dest.cc >> > index cb818708aef..372ed6e0c00 100644 >> > --- a/libstdc++-v3/testsuite/experimental/net/timer/waitable/dest.cc >> > +++ b/libstdc++-v3/testsuite/experimental/net/timer/waitable/dest.cc >> > @@ -18,6 +18,7 @@ >> > // { dg-do run { target c++14 } } >> > // { dg-add-options libatomic } >> > // { dg-xfail-if "poll not available" { *-*-rtems* } } >> > +// { dg-require-thread-fence "" } // needed by >> > __gnu_debug::_Safe_sequence_base::_M_detach_all() >> > >> > #include <experimental/timer> >> > #include <testsuite_hooks.h> >> > diff --git a/libstdc++-v3/testsuite/experimental/net/timer/waitable/ops.cc >> > b/libstdc++-v3/testsuite/experimental/net/timer/waitable/ops.cc >> > index ae51979c3b4..8383e0be6a4 100644 >> > --- a/libstdc++-v3/testsuite/experimental/net/timer/waitable/ops.cc >> > +++ b/libstdc++-v3/testsuite/experimental/net/timer/waitable/ops.cc >> > @@ -18,6 +18,7 @@ >> > // { dg-do run { target c++14 } } >> > // { dg-add-options libatomic } >> > // { dg-xfail-if "poll not available" { *-*-rtems* } } >> > +// { dg-require-thread-fence "" } // needed by >> > __gnu_debug::_Safe_sequence_base::_M_detach_all() >> > >> > #include <experimental/timer> >> > #include <testsuite_hooks.h> >> > diff --git >> > a/libstdc++-v3/testsuite/experimental/polymorphic_allocator/construct_pair.cc >> > >> > b/libstdc++-v3/testsuite/experimental/polymorphic_allocator/construct_pair.cc >> > index 960c1d253b5..42de45619a8 100644 >> > --- >> > a/libstdc++-v3/testsuite/experimental/polymorphic_allocator/construct_pair.cc >> > +++ >> > b/libstdc++-v3/testsuite/experimental/polymorphic_allocator/construct_pair.cc >> > @@ -16,6 +16,7 @@ >> > // <http://www.gnu.org/licenses/>. >> > >> > // { dg-do run { target c++14 } } >> > +// { dg-require-thread-fence "" } // needed by >> > __gnu_debug::_Safe_sequence_base::_M_detach_all() >> > >> > #include <experimental/memory_resource> >> > #include <utility> >> >> >> I'm concerned with how much of the testsuite is being completely >> disabled for this target. > > I think that depends on which "flavour" of arm-eabi we are looking at. > I'm trying to cleanup some of the "obvious" failures, and that proves to be > tedious: > - quite a few gcc/g++ tests have problems when overriding -mcpu/-march with > dejagnu's target-board > - so I thought a first step could be to clean the default configuration, but > as said above it targets a very old architecture > >> >> >> Any tests with "debug" in the path are probably relying on the debug >> mode, and any that use -D_GLIBCXX_DEBUG in dg-options are. And I >> suppose it's expected that 29_atomics/* tests rely on atomic >> synchronization, but it's unfortunate that those now can't be tested >> for arm-eabi, and I don't understand why it only affects eight of the > > that's only the "default" arm-eabi, it does not happen in other non-default > arm-eabi (well I didn't check all possible cases) > >> >> atomics tests not all the other ones. >> >> Something doesn't seem right here. >> > > Thanks, > > Christophe >
