libstdc++: '_GLIBCXX_HAS_GTHREADS' -- '__GTHREADS' vs. '__GTHREADS_CXX0X'

[Jonathan Wakely]

On Thu, 20 Feb 2025 at 13:53, Thomas Schwinge <tschwinge@baylibre.com> wrote:
>
> Hi Jonathan!
>
> On 2025-02-20T12:51:15+0000, Jonathan Wakely <jwakely@redhat.com> wrote:
> > On Thu, 20 Feb 2025 at 12:29, Thomas Schwinge <tschwinge@baylibre.com> wrote:
> >> I'm still working on libstdc++ support for GCC's GPU targets: GCN, nvptx.
> >> These configurations are (in typical use) multi-threaded, and support
> >> mutexes etc., but they don't support dynamic spawning of threads etc.
> >> Therefore, re 'libgcc/gthr.h', they define '__GTHREADS' but *not*
> >> '__GTHREADS_CXX0X'.  (See 'libgcc/config/gcn/gthr-gcn.h'; nvptx still to
> >> be done, currently 'Thread model: single', very likely bogus...)
> >>
> >> Now, 'libstdc++-v3/acinclude.m4:GLIBCXX_CHECK_GTHREADS' does:
> >>
> >>     [...]
> >>       AC_MSG_CHECKING([for gthreads library])
> >>
> >>       AC_TRY_COMPILE([#include "gthr.h"],
> >>         [
> >>           #ifndef __GTHREADS_CXX0X
> >>           #error
> >>           #endif
> >>         ], [ac_has_gthreads=yes], [ac_has_gthreads=no])
> >>       else
> >>         ac_has_gthreads=no
> >>       fi
> >>
> >>       AC_MSG_RESULT([$ac_has_gthreads])
> >>
> >>       if test x"$ac_has_gthreads" = x"yes"; then
> >>         AC_DEFINE(_GLIBCXX_HAS_GTHREADS, 1,
> >>                   [Define if gthreads library is available.])
> >>     [...]
> >>
> >> That is, it defines '_GLIBCXX_HAS_GTHREADS' per '__GTHREADS_CXX0X'.
> >> Dependent on this '_GLIBCXX_HAS_GTHREADS',
> >> 'libstdc++-v3/include/bits/std_mutex.h' then enables 'class mutex'.  In
> >> other words, in the current GCN configuration ('__GTHREADS', but not
> >> '__GTHREADS_CXX0X'), 'class mutex' (and probably more) is not available,
> >> which leads to build issues (and presumably a lot of noise in the test
> >> suite).
> >
> > What are the build issues? There shouldn't be any, because we should
> > be checking _GLIBCXX_HAS_GTHREADS as needed.
>
> The build issue is 'libstdc++-v3/src/c++20/tzdb.cc', which checks
> '#if defined __GTHREADS' (which GCN defines), and then assumes that
> 'mutex' is available,

Right, that's a bug (and not the first time I've done that).

> but that one via '_GLIBCXX_HAS_GTHREADS' is guarded
> on '__GTHREADS_CXX0X' (which GCN doesn't define).
>
> Now clearly 'std::chrono::tzdb' is not going to be used in offloaded code
> regions, but (a) famous last words..., and (b) I was more worried about
> the general case, where code (user code or libstdc++-internal) would see
> '!_GLIBCXX_HAS_GTHREADS', and from that deduce that locking for
> concurrent access etc. is not necessary.
>
> > And the testsuite should use dg-require-gthreads.
>
> > So instead of trying to make the C++11 thread library work without the
> > necessary gthreads support
>
> Right, we're not attempting to make 'std::thread::detach' work, for
> example.  But I was assuming that we could/had to make 'std::mutex'
> etc. work, so that algorithms in presence of "external parallelism" still
> do the appropriate locking etc.
>
> Like, when used within a GPU parallel kernel, 'std::chrono::tzdb' needs
> to 'mutex' its internal state, thus, guarding that on '__GTHREADS' would
> seem correct, even if '!__GTHREADS_CXX0X'?

Yes. Elsewhere in the library we do that with __gnu_cxx::__mutex
instead of std::mutex.

I think this should fix it:

--- a/libstdc++-v3/src/c++20/tzdb.cc
+++ b/libstdc++-v3/src/c++20/tzdb.cc
@@ -48,6 +48,7 @@
#else
# define USE_ATOMIC_LIST_HEAD 0
# define USE_ATOMIC_SHARED_PTR 0
+# include <ext/concurrence.h>
#endif

#if USE_ATOMIC_SHARED_PTR && ! USE_ATOMIC_LIST_HEAD
@@ -101,6 +102,8 @@ namespace std::chrono
#ifndef __GTHREADS
    // Dummy no-op mutex type for single-threaded targets.
    struct mutex { void lock() { } void unlock() { } };
+#elif ! defined _GLIBCXX_HAS_GHTREADS
+    using mutex = __gnu_cxx::__mutex;
#endif
    inline mutex& list_mutex()
    {


We could also enable std::mutex for the __GTHREADS && !
__GTHREADS_CXX0X case but that should probably wait for stage 1.



>
> > maybe you just need to identify a handful
> > of bugs where we aren't checking the right macros in the libstdc++
> > sources?
> >
> >> Is '__GTHREADS != __GTHREADS_CXX0X' simply a configuration that libstdc++
> >> so far has not attempted to support?  To make that work, do we need to
> >> consider '__GTHREADS' vs. '__GTHREADS_CXX0X' individually in libstdc++,
> >> for guarding the respective features?
>
> Another option for GCN, nvptx -- and maybe that's actually easiest: we
> define '__GTHREADS_CXX0X' in addition to '__GTHREADS', and implement the
> additional interfaces as "operation is not supported, -1 is returned"?
> That should make libstdc++ "happy" and give a consistent user experience
> (like, when the OS is out of resources)?
>
>
> Grüße
>  Thomas
>