[cfe-commits] C11 <stdatomic.h>

[Richard Smith]

On Wed, Oct 3, 2012 at 8:54 AM, Tijl Coosemans <tijl at coosemans.org> wrote:

> >> On Tue, Sep 18, 2012 at 6:47 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
> >>> On Mon, Sep 17, 2012 at 4:11 PM, Eli Friedman <eli.friedman at gmail.com>
> wrote:
> >>>> On Sat, Sep 15, 2012 at 1:05 AM, Richard Smith <richard at metafoo.co.uk
> >
> >>>> wrote:
> >>>>> On Fri, Sep 14, 2012 at 8:17 PM, Eli Friedman <
> eli.friedman at gmail.com>
> >>>>> wrote:
> >>>>>> On Fri, Sep 14, 2012 at 7:48 PM, Richard Smith <
> richard at metafoo.co.uk
> >>>>>> wrote:
> >>>>>>> The attached patch adds an implementation of
> >>>>>>> <stdatomic.h> to the set of headers provided by Clang.
> >>>>>>> Since this header is so compiler-dependent, it seems that
> >>>>>>> we are the most rational component to be providing this
> >>>>>>> header (even though, for instance, some flavors of BSD
> >>>>>>> already provide their own).
> >>>>>>> Please review!
> >>>>>>
> >>>>>> +// Clang allows memory_order_consume ordering for
> __c11_atomic_store,
> >>>>>> +// even though C11 doesn't allow it for atomic_store.
> >>>>>>
> >>>>>> That looks like a bug...
> >>>>>
> >>>>> Possibly it's a bug in the specification for atomic_flag_clear?
> >>>>> memory_order_consume doesn't seem to have any meaning for a store
> >>>>> operation.
> >>>>>
> >>>>>> Please put the new warning in a separate commit.
> >>>>>
> >>>>> r163964.
> >>>>>
> >>>>>> It looks like standard requires that we expose functions named
> >>>>>> atomic_thread_fence, atomic_signal_fence, atomic_flag_test_and_set,
> >>>>>> atomic_flag_test_and_set_explicit, and atomic_flag_clear; your
> version
> >>>>>> of stdatomic.h doesn't include declarations for these functions
> (which
> >>>>>> is required by C11 7.1.4p1).
> >>>>>
> >>>>> Ugh. And C11 7.1.2/6 requires them to have external linkage.
> >>>>> I don't want these functions to require linking to a library.
> >>>>> We could emit them weak and inline, but then we'll get a weak
> >>>>> copy in every TU which includes this header, which seems
> >>>>> fairly egregious. Is there currently any way to emit a
> >>>>> function as linkonce_odr from C? Do you have any suggestions
> >>>>> as to how to proceed?
> >>>>
> >>>> There isn't any way to get linkonce_odr from C at the moment; patches
> >>>> welcome.  I don't see any issues with that from the standpoint of the
> >>>> standard; I'm a little worried about ABI-compat issues, though.
> >>>> (Specifically, if the system provides the header, having our own
> >>>> linkonce_odr version could cause strange linker errors.)
> >>>>
> >>>> We could put it into compiler-rt, and say that if someone tries to use
> >>>> the function instead of the macro without linking in compiler-rt,
> >>>> that's an error.  Not particularly satisfying either, but somewhat
> >>>> simpler.
> >>>
> >>> After some discussion with Chandler, we think the best approach is to
> say
> >>> that the definition of these functions belongs in libc, and to provide
> only
> >>> declarations of them. A patch for that approach is attached.
>
> If the compiler provides stdatomic.h but libc implements the functions
> then all compilers need to have a binary compatible definition of
> memory_order and atomic_flag types.
>

[On the assumption that these atomic operations, and stdatomic.h itself,
should be provided by the compiler rather than by libc...] The definition
of atomic_flag needs to be part of the ABI anyway, if we want to have any
hope of compiler interoperability with (structs containing) atomic types,
so I view that part as a non-issue. The memory_order part is unfortunate,
though a libc could choose to ignore that parameter and always treat it as
seq_cst (since this only affects situations where the macro definition
isn't being used, it should be very rare).

The patch implements atomic_flag on top of atomic_bool, but that means
> atomic_flag f = ATOMIC_FLAG_INIT is an atomic store.


Not true. There is no need for the initialization of an _Atomic variable to
use an atomic write, and the code Clang emits does not perform one.


> Also instead of including stdint.h and stddef.h, maybe you can use
> preprocessor macros like __INT8_TYPE__.
>

Much of the work in stdint.h goes into defining the int_least*_t and
int_fast*_t types correctly, and it doesn't make sense to duplicate that
when defining atomic_int_least*_t etc. I don't think removing these
dependencies is particularly worthwhile.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121003/28afb428/attachment.html>