[PR c/68966] Restore atomic builtins usage in libstdc++-v3

[Thomas Schwinge]

Hi!

On Mon, 21 Mar 2016 15:01:49 +0000, Jonathan Wakely <jwakely@redhat.com> wrote:
> 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 ...

(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.
> 
> 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.

"Don't shoot the messenger", who quickly runs away to stay out of this
mess.  ;-)


Grüße
 Thomas