Re: [RFA] choosing __platform_wait_t on targets without lock-free 64 atomics

[Jonathan Wakely <jwakely.gcc@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2210 bytes --]

On Thu, 29 Dec 2022, 11:29 Iain Sandoe, <iain@sandoe.co.uk> wrote:

> Hi,
>
> The recent addition of the tz handling has pulled in a dependency on
> </bits/atomic_wait.h>
>
> This currently specifies __platform_wait_t as a 64bit quatity on platforms
> without _GLIBCXX_HAVE_LINUX_FUTEX.
>
> PowerPC does not have a 64b atomic without library support - so that this
> causes a bootstrap
> fail on powerpc-darwin (and I guess any other 32b powerpc non-futex
> target).
>
> Rather than contrive to build and add libatomic (which is not at present
> available at the point
> that libstdc++ is built), I wonder if there is any specific reason that
> __platform_wait_t needs
> to be 64 bits on these platforms? (Especially since the futex case uses an
> int.)
>

I think we do want the generic case's _M_wait atomic variable to be lock
free, otherwise we use two locks for every operation, the one in libatomic
and the waiter mutex. That's more important than it being any specific
width.


> Advice on the right way to fix this welcome — as a work-around to allow
> bootstrap to complete
> I applied the patch below - but that seems unlikely to be the right thing
> generically .


Rather than __lp64__ I think we should check the ATOMIC_LONG_LOCK_FREE
macro and use long if it's lock free and int otherwise. But Tom needs to
confirm that. That would be approximately the same as your patch in
practice.






> thanks
> Iain
>
> ----
>
>
> diff --git a/libstdc++-v3/include/bits/atomic_wait.h
> b/libstdc++-v3/include/bits/atomic_wait.h
> index bd1ed56..2f67180 100644
> --- a/libstdc++-v3/include/bits/atomic_wait.h
> +++ b/libstdc++-v3/include/bits/atomic_wait.h
> @@ -64,7 +64,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  // and __platform_notify() if there is a more efficient primitive
> supported
>  // by the platform (e.g. __ulock_wait()/__ulock_wake()) which is better
> than
>  // a mutex/condvar based wait.
> +#if __LP64__
>      using __platform_wait_t = uint64_t;
> +#else
> +    using __platform_wait_t = uint32_t;
> +#endif
>      inline constexpr size_t __platform_wait_alignment
>        = __alignof__(__platform_wait_t);
>
>