From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x630.google.com (mail-ej1-x630.google.com [IPv6:2a00:1450:4864:20::630]) by sourceware.org (Postfix) with ESMTPS id 6FE533858D20 for ; Thu, 29 Dec 2022 17:02:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6FE533858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-ej1-x630.google.com with SMTP id qk9so46333245ejc.3 for ; Thu, 29 Dec 2022 09:02:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=PMa8DoRdRcrq3GizSl2Hquc3itTBgqnTM0uZ3Tp3RpY=; b=oPJP20HyWBYyvvZdQEljNxS40DqBXohN7M+q9oZ48k8XtmRhmsjJEzye8MKMPRSMMJ ASuypIGkojfDfh5WzQtcgBbT9CXDSCSRMJz8h1oMni9mdVnrJExclmkXdzSDF7vNODcK rm+EQN1XVFAaG0HGPkjZhpmj3XMvvcTLAJrQsxbmJg5hS2qo3QwQHmnIlOQszX5n+aPm GhJlYKEXwVrYW7k96lb6xS7QyOdeKtwBSMih9VPmfmf/xty0b5b3Gn/K2sAhb9cn/ZTA HAVDLTGIPbOg2AHCAHUc69/leEOfvTGCLkk4Abwmh1i6fmeYt9RGjPMsWIPGh96WdD5i OPyQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=PMa8DoRdRcrq3GizSl2Hquc3itTBgqnTM0uZ3Tp3RpY=; b=jPmsL7KiNfLDlrJO1jiuYi7fGjCFU2iTCTnAHw0NKWPrOC7kffrHWY9/MSXeaVJWMF KFNNj/zaIyCcECRKBnVd2vOwlYC0W24/en3Ks9KctTi1LEPiOOhcArpxlEm5mb1pzsr7 DFUv2Ht7LG6Js//0HmY2SxvJjcsUmrKgzg6OF/mCrKrAnbkAwEaE9TzaeyBI11x07XQV KORdVfewREXdXpdGtbL35n76JRqfWF1xTqSJAEnAPsDHNRsB6zkd6/VzTmCVPqZs99oX /tFv7T00rzjfou+ID4UJh3GJ/yupdiq/vZYyQ3lH6XvISzAnLjAr/jfoThnrgHKjE5fF JIYw== X-Gm-Message-State: AFqh2kqBK4lDvv4YDCOQxT15hQWbz2R4pvpT+nIGkW/Wb77tzBrHAixi A5W8pnEYTKYedRU/eBVOCxI1apvSqletykRYZ+U= X-Google-Smtp-Source: AMrXdXvKAM2pfY8Z2zyCSjYL5JxPLM4hh34FQI38XONOYOZTNSBW/jXHU5BTTIxkvqtbbkqhg4lyecJdpqiJGmMDl4o= X-Received: by 2002:a17:906:9243:b0:7c0:d58f:3409 with SMTP id c3-20020a170906924300b007c0d58f3409mr1556629ejx.416.1672333370271; Thu, 29 Dec 2022 09:02:50 -0800 (PST) MIME-Version: 1.0 References: <6640F26B-F267-40E0-9223-2F0F45462176@sandoe.co.uk> <05A9C1EF-7EC9-4276-BA11-8922A66B3D3A@sandoe.co.uk> In-Reply-To: From: Jonathan Wakely Date: Thu, 29 Dec 2022 17:02:39 +0000 Message-ID: Subject: Re: [RFA] choosing __platform_wait_t on targets without lock-free 64 atomics To: Iain Sandoe Cc: "libstdc++" , Thomas Rodgers Content-Type: multipart/alternative; boundary="0000000000007b2c5805f0fa747c" X-Spam-Status: No, score=-6.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,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: --0000000000007b2c5805f0fa747c Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 29 Dec 2022, 16:22 Iain Sandoe, wrote: > > > > On 29 Dec 2022, at 15:44, Jonathan Wakely wrote: > > > > > > > > On Thu, 29 Dec 2022, 15:30 Iain Sandoe, wrote: > > 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 > > > >> > > >> 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. > > > > Definitely, that=E2=80=99s probably a recipe for some subtle race condi= tion .. > nested locks etc. > > > > I didn't see any nested cases from a quick look, but it would still be > better to avoid two locks. > > > > > > >> Advice on the right way to fix this welcome =E2=80=94 as a work-arou= nd 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. > > > > OK.. that makes sense here=E2=80=99s a proposed patch (pending subseque= nt 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. > > > > Agreed. > > > > > > 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. > > > > Yep, but do we need the size checks at all? > > > > 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. > > That=E2=80=99s fine by me - I was just copying what was there :) > > In this patch I made it so that a target without a =E2=80=98suitable' loc= k-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 > 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. > Iain > > > > > 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 > > --0000000000007b2c5805f0fa747c--