From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp001-out.apm-internet.net (smtp001-out.apm-internet.net [85.119.248.222]) by sourceware.org (Postfix) with ESMTPS id D56ED3858D32 for ; Mon, 2 Jan 2023 07:47:43 +0000 (GMT) Received: (qmail 15597 invoked from network); 2 Jan 2023 07:47:42 -0000 X-APM-Out-ID: 16726456621559 X-APM-Authkey: 257869/1(257869/1) 3 Received: from unknown (HELO smtpclient.apple) (81.138.1.83) by smtp001.apm-internet.net with SMTP; 2 Jan 2023 07:47:42 -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: Mon, 2 Jan 2023 07:47:42 +0000 Cc: Jonathan Wakely , libstdc++ Content-Transfer-Encoding: quoted-printable Message-Id: <100B3504-8A57-42C0-B607-758374766869@sandoe.co.uk> References: <6640F26B-F267-40E0-9223-2F0F45462176@sandoe.co.uk> <05A9C1EF-7EC9-4276-BA11-8922A66B3D3A@sandoe.co.uk> <645C69D4-8147-4EAF-BCD9-42CA0C84E28B@sandoe.co.uk> To: Thomas Rodgers X-Mailer: Apple Mail (2.3696.120.41.1.1) X-Spam-Status: No, score=-8.1 required=5.0 tests=BAYES_00,KAM_COUK,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: > On 2 Jan 2023, at 00:53, Thomas Rodgers wrote: >=20 > __platform_wait_t should be whatever the platform supports lock free = natively. The use of a 64 bit int there in the fall through was copied = from Olivier's original implementation for libc++, which uses = __ulock_wait/wake on Darwin which takes a unit64_t, because I had = intended to add support Darwin, but haven't done so yet. In general, libc++ supports fewer versions of Darwin than GCC does (I = don=E2=80=99t know right now which versions/archs we support have the = __ulock_wait/wake). Of course, I would very much like to see an = efficient solution for Darwin - so please let me know/share patches with = me when you get to it - I can test on older supported versions. However, the issue here is not really Darwin-specific - it will effect = any target that does not have either a futex or a 64b lock-free atomic. thanks Iain >=20 > On Fri, Dec 30, 2022 at 2:51 AM Iain Sandoe wrote: >=20 >=20 > > On 29 Dec 2022, at 17:02, Jonathan Wakely = wrote: > >=20 > >=20 > >=20 > > On Thu, 29 Dec 2022, 16:22 Iain Sandoe, wrote: > >=20 > >=20 > > > On 29 Dec 2022, at 15:44, Jonathan Wakely = wrote: > > >=20 > > >=20 > > >=20 > > > On Thu, 29 Dec 2022, 15:30 Iain Sandoe, wrote: > > > Hi Folks, > > >=20 > > > > On 29 Dec 2022, at 12:09, Jonathan Wakely = wrote: > > >=20 > > > > On Thu, 29 Dec 2022, 11:29 Iain Sandoe, = wrote: > > >=20 > > > >> 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. > > >=20 > > > Definitely, that=E2=80=99s probably a recipe for some subtle race = condition .. nested locks etc. > > >=20 > > > I didn't see any nested cases from a quick look, but it would = still be better to avoid two locks. > > >=20 > > >=20 > > > >> 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. > > >=20 > > > OK.. that makes sense here=E2=80=99s a proposed patch (pending = subsequent input from Tom). > > >=20 > > > 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. > > >=20 > > > Agreed. > > >=20 > > >=20 > > > 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. > > >=20 > > > Yep, but do we need the size checks at all? > > >=20 > > > I was thinking we could just use 'unsigned long' or 'unsigned int' = directly, instead of a uintN_t typedef. Using the typedef just seems to = complicate things. > >=20 > > That=E2=80=99s fine by me - I was just copying what was there :) > >=20 > > In this patch I made it so that a target without a =E2=80=98suitable' = lock-free size would fail to > > compile the source, which seems better than a link fail later =E2=80=94= I could make it more > > specific (e.g. # fail clause) or we could test for smaller lock-free = entities=E2=80=A6 > >=20 > > I think we can just eschew atomics altogether in that case, and just = use the mutex for all accesses. I can do that after the break when I'm = back online. >=20 > Great, thanks! > cheers > Iain >=20 > I=E2=80=99m using this locally in the meantime: >=20 > # if ATOMIC_LONG_LOCK_FREE =3D=3D 2 > using __platform_wait_t =3D long; > # elif ATOMIC_INT_LOCK_FREE =3D=3D 2 > using __platform_wait_t =3D int; > # elif ATOMIC_SHORT_LOCK_FREE =3D=3D 2 > using __platform_wait_t =3D short; > # elif ATOMIC_CHAR_LOCK_FREE =3D=3D 2 > using __platform_wait_t =3D char; > # else > # fail No suitable lock-free type found. > # endif >=20