From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp002.apm-internet.net (smtp002.apm-internet.net [85.119.248.221]) by sourceware.org (Postfix) with ESMTPS id E073A3858D20 for ; Thu, 29 Dec 2022 15:30:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E073A3858D20 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=sandoe.co.uk Authentication-Results: sourceware.org; spf=none smtp.mailfrom=sandoe.co.uk Received: (qmail 80773 invoked from network); 29 Dec 2022 15:30:05 -0000 X-APM-Out-ID: 16723278058077 X-APM-Authkey: 257869/1(257869/1) 5 Received: from unknown (HELO smtpclient.apple) (81.138.1.83) by smtp002.apm-internet.net with SMTP; 29 Dec 2022 15:30:05 -0000 Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.1\)) Subject: Re: [RFA] choosing __platform_wait_t on targets without lock-free 64 atomics From: Iain Sandoe In-Reply-To: Date: Thu, 29 Dec 2022 15:30:05 +0000 Cc: Thomas Rodgers , Jonathan Wakely Content-Transfer-Encoding: quoted-printable Message-Id: <05A9C1EF-7EC9-4276-BA11-8922A66B3D3A@sandoe.co.uk> References: <6640F26B-F267-40E0-9223-2F0F45462176@sandoe.co.uk> To: libstdc++ X-Mailer: Apple Mail (2.3696.120.41.1.1) X-Spam-Status: No, score=-14.1 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_COUK,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_NONE,TXREP 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: Hi Folks, > On 29 Dec 2022, at 12:09, Jonathan Wakely = wrote: > On Thu, 29 Dec 2022, 11:29 Iain Sandoe, wrote: >> The recent addition of the tz handling has pulled in a dependency on = >>=20 >> This currently specifies __platform_wait_t as a 64bit quatity on = platforms without _GLIBCXX_HAVE_LINUX_FUTEX. >>=20 >> 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). >>=20 >> 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.) >>=20 > 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. Definitely, that=E2=80=99s probably a recipe for some subtle race = condition .. nested locks etc. >> Advice on the right way to fix this welcome =E2=80=94 as a = work-around to allow bootstrap to complete >> I applied the patch below - but that seems unlikely to be the right = thing generically . >>=20 > 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. OK.. that makes sense here=E2=80=99s a proposed patch (pending = subsequent input from Tom). I am using =E2=80=9Clock free always=E2=80=9D as the criterion, = =E2=80=9Csometimes=E2=80=9D does not seem useful here. Although we normally build libstdc++ with the just-built GCC... .. AFAIK the __SIZEOF_* are available from any version of GCC or clang = that would be capable of building the sources. The contortion in the first #elif is attempting to cater for the = (supposed, possible) case that there could be a 16b int target with a lock free 32b long (that = might be a stretch so feel free to delete the second half). .. wider testing will follow, smoke tested only ... cheers Iain =3D=3D=3D=3D=3D=3D=3D=3D diff --git a/libstdc++-v3/include/bits/atomic_wait.h = b/libstdc++-v3/include/bits/atomic_wait.h index bd1ed56..3ef0e92 100644 --- a/libstdc++-v3/include/bits/atomic_wait.h +++ b/libstdc++-v3/include/bits/atomic_wait.h @@ -64,7 +64,15 @@ _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 (__SIZEOF_LONG__ =3D=3D 8 && ATOMIC_LONG_LOCK_FREE =3D=3D 2) || \ + (__SIZEOF_LONG_LONG__ =3D=3D 8 && ATOMIC_LLONG_LOCK_FREE =3D=3D 2) using __platform_wait_t =3D uint64_t; +# elif (__SIZEOF_INT__ =3D=3D 4 && ATOMIC_INT_LOCK_FREE =3D=3D 2) || \ + (__SIZEOF_LONG__ =3D=3D 4 && ATOMIC_LONG_LOCK_FREE =3D=3D 2) + using __platform_wait_t =3D uint32_t; +# elif (__SIZEOF_INT__ =3D=3D 2 && ATOMIC_INT_LOCK_FREE =3D=3D 2) + using __platform_wait_t =3D uint16_t; +# endif inline constexpr size_t __platform_wait_alignment =3D __alignof__(__platform_wait_t); #endif