From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 122934 invoked by alias); 21 Mar 2016 16:02:07 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 122873 invoked by uid 89); 21 Mar 2016 16:02:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=shoot, sk:jwakely X-Spam-User: qpsmtpd, 2 recipients X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 21 Mar 2016 16:02:04 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-01.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1ai2HJ-0006WO-RE from Thomas_Schwinge@mentor.com ; Mon, 21 Mar 2016 09:02:02 -0700 Received: from hertz.schwinge.homeip.net (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.3.224.2; Mon, 21 Mar 2016 16:02:00 +0000 From: Thomas Schwinge To: Jonathan Wakely CC: Martin Sebor , , , Marek Polacek , "Joseph S. Myers" , Jeff Law Subject: Re: [PR c/68966] Restore atomic builtins usage in libstdc++-v3 In-Reply-To: <20160321150149.GK3805@redhat.com> References: <567A271B.2090403@gmail.com> <87zitsqbho.fsf@kepler.schwinge.homeip.net> <20160321150149.GK3805@redhat.com> User-Agent: Notmuch/0.9-101-g81dad07 (http://notmuchmail.org) Emacs/24.4.1 (x86_64-pc-linux-gnu) Date: Mon, 21 Mar 2016 16:04:00 -0000 Message-ID: <87zitrbz0o.fsf@hertz.schwinge.homeip.net> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-SW-Source: 2016-03/txt/msg01189.txt.bz2 Hi! On Mon, 21 Mar 2016 15:01:49 +0000, Jonathan Wakely wr= ote: > On 21/03/16 13:08 +0100, Thomas Schwinge wrote: > >Per my (admittedly, not in-depth) reading of libstdc++-v3 source code, > >the _GLIBCXX_ATOMIC_BUILTINS conditional is only used in combination with > >the _Atomic_word data type, which in > >libstdc++-v3/doc/xml/manual/concurrency_extensions.xml is described as "a > >signed integral type" (so, matching the semantics as clarified by your > >patch). That makes sense: it's used to keep reference counts, for > >example. So, it seems sound to just remove the bool atomics check. >=20 > I agree that it doesn't make any sense to check whether atomics work > for bool when we only care about them for _Atomic_word, however ... (Please review that it really is used only for that; I have only done a quick scan of the libstdc++-v3 sources.) > This would change the value of _GLIBCXX_ATOMIC_BUILTINS for any target > which was already failing the check for bool but passing it for the > other types. We would now switch to using atomic builtins where we > previously didn't use them, which could be a problem. I don't know if > there are any targets that would be affected, and if it would cause an > actual problem. Assuming there are no other reasons that could have caused the bool atomics checks to fail (under the condition that the short and int ones did and still do succeed), my patch just restores the state of a few months ago, before Martin's bool atomics warning patch got committed. So, I think it is safe to commit. > Would leaving the bool check in place, but just removing the > __atomic_fetch_add() part be better? It should still fix the > regression, but is less likely to change behaviour for targets that > were never using the builtins. Yes, we could do that, but while I have not verified this, I assume that it's very unlikely that there exists a configuration where the bool atomics checks already used to fail but the short and int ones did and still do succeed. Anyway, that's not my decision to make. ;-) > >That said, I have not looked into why the libstdc++-v3 configure script > >checks short, int, and long long atomics, but not long. But then, only > >the short and int results are used to decide on _GLIBCXX_ATOMIC_BUILTINS, > >and on the other hand there actually are "typedef long _Atomic_word" > >definitions, but long atomics are not explicitly checked for. >=20 > Ugh. Those checks have a long and messy history. >=20 > If we're now only using _GLIBCXX_ATOMIC_BUILTINS to decide how to > operate on _Atomic_word then it should really only be testing whether > atomics are supported for that type. I'm not brave enough to change > that now though. "Don't shoot the messenger", who quickly runs away to stay out of this mess. ;-) Gr=C3=BC=C3=9Fe Thomas