[cfe-dev] RFC: __atomic_* support for gcc 4.7 compatibility

[David Chisnall]

On 10 Apr 2012, at 20:22, Richard Smith wrote:

> Hi,
> 
> I'm looking into adding support for gcc 4.7's __atomic_* builtins for the clang 3.1 release (these are necessary to support various parts of libstdc++4.7). Our current set of __atomic_* builtins is incompatible with gcc 4.7's builtins in a few ways:

While it would be nice to build libstdc++, it should be noted that the gcc builtins are poorly designed in a number of ways and I don't think we should be encouraging their use by supporting them in anything other than the specific case of supporting libstdc++.   The clang ones are a closer match for the C11 spec (and we should really be encouraging people to use stdatomic.h, rather than using the intrinsics directly anyway).

> 1) In clang, the builtins produce results by value, whereas in gcc, a pointer is passed indicating where the result should be written.

This means that the things required by C11's stdatomic.h can be trivially implemented as macros (just dropping the __ prefix) with the clang builtins.  They can not with the GCC ones in their standard form, although the _n versions can be used this way.  I am not sure what the perceived advantage of the GCC non-_n ones is.  

> 2) In clang, the argument type must be an _Atomic. gcc does not have this restriction (but also fails to require that the type is trivially copyable, which is probably a bug).

This is a horrible bug in GCC, because it means that you can have atomic and non-atomic accesses to the same memory address (something explicitly not allowed by C11).  This is part of the reason for the _Atomic() qualifier.  It also changes the alignment requirements of certain types, in both gcc and clang, which can lead to really hard-to-track-down bugs as a result of sloppy casting.  

I would strongly oppose supporting using the builtins with types that are not _Atomic-qualified, because doing so makes it trivial to introduce a large number of very subtle bugs.

> 3) Clang is missing the __atomic_*_n, __atomic_always_lock_free and __atomic_fetch_nand builtins.

The _n builtins actually do what the current clang ones do.  __atomic_always_lock_free is probably useful.  __atomic_fetch_nand might be as well, although it isn't needed by C++11 or C11.

> 4) Clang is missing the __atomic_compare_exchange builtin, and instead provides __atomic_compare_exchange_strong and __atomic_compare_exchange_weak.

I'm not sure what the point of the GCC version is.  The strong / weak option affects code generation, it's not something that can be selected at run time, so having a bool parameter just makes code less readable.

> 5) Clang provides an __atomic_init builtin, which gcc does not provide, and which crashes during CodeGen.

This is required for C11.  I'll take a look at the crash.  It's also used by libc++.  __atomic_init() initialises an _Atomic type without performing an atomic operation (as per both C++11 and C11).  Or, at least, it should...

I believe that libstdc++ gets around this by not _Atomic-qualifying the types and then using normal initialisation.  This is... problematic, to say the least, as an approach.  

> My current plan is:
> 
> For (1), add support for the gcc-style arguments, then port libc++ over to them, then remove support for the current arguments.

Please don't.  

> For (2), relax the requirement to the type being either _Atomic or trivially copyable.

I think this would be a serious mistake.  Allowing atomic operations on non-_Atomic types in the intrinsics makes it insanely hard to correctly implement C11 stdatomic.h and will cause all sorts of problems for users of these builtins later.

> For (3) and (4), add the missing builtins. I don't intend to remove the __atomic_compare_exchange_strong/__atomic_compare_exchange_weak builtins, but will do so if anyone particularly wants to see them gone.

As with the other clang builtins, they make stdatomic.h much easier to implement.  From FreeBSD's stdatomic.h:

#if defined(__CLANG_ATOMICS)
#define atomic_compare_exchange_strong_explicit(object, expected,       \
    desired, success, failure)                                          \
        __atomic_compare_exchange_strong(object, expected, desired,     \
            success, failure)
#define atomic_compare_exchange_weak_explicit(object, expected,         \
    desired, success, failure)                                          \
        __atomic_compare_exchange_weak(object, expected, desired,       \
            success, failure)

Replacing these with a bool parameter would probably be okay, but it makes the code less readable.  Which of these do you find easier to understand?

__atomic_compare_exchange_strong(&a, &new, 12, __ATOMIC_SEQ_CST, __ATOMIC_RELAXED);

Or:

__atomic_compare_exchange_strong(&a, &new, 12, true, __ATOMIC_SEQ_CST, __ATOMIC_RELAXED);

What is that true for?  What happens if it's a variable?  I honestly have no idea - switching between strong and weak implementations at run time is more or less impossible.

> For (5), remove this builtin.

Again, please don't, it is required for C11 and makes achieving the correct semantics for C++11 much easier.

> Please shout now if any part of this displeases you! In particular, if you have code depending on clang's current pass-by-value atomics and you'd like to see them left in until after the clang 3.1 release to give you more time to move to the portable forms (or indeed you believe they should be left in-place forever), please say so.

See above.

David

-- Sent from my Cray X1