This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PR c/68966] Restore atomic builtins usage in libstdc++-v3 (was: [PATCH] c/68966 - atomic_fetch_* on atomic_bool not diagnosed)
- From: Jonathan Wakely <jwakely at redhat dot com>
- To: Thomas Schwinge <thomas at codesourcery dot com>
- Cc: Martin Sebor <msebor at gmail dot com>, libstdc++ at gcc dot gnu dot org, gcc-patches at gcc dot gnu dot org, Marek Polacek <polacek at redhat dot com>, "Joseph S. Myers" <joseph at codesourcery dot com>, Jeff Law <law at redhat dot com>
- Date: Mon, 21 Mar 2016 15:01:49 +0000
- Subject: Re: [PR c/68966] Restore atomic builtins usage in libstdc++-v3 (was: [PATCH] c/68966 - atomic_fetch_* on atomic_bool not diagnosed)
- Authentication-results: sourceware.org; auth=none
- References: <567A271B dot 2090403 at gmail dot com> <87zitsqbho dot fsf at kepler dot schwinge dot homeip dot net>
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.
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 ...
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.
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.
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.
Ugh. Those checks have a long and messy history.
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.