[cfe-commits] C11 <stdatomic.h>

[Richard Smith]

On Thu, Oct 4, 2012 at 12:23 PM, Richard Smith <richard at metafoo.co.uk>wrote:

> On Thu, Oct 4, 2012 at 5:18 AM, Tijl Coosemans <tijl at coosemans.org> wrote:
>
>> On 03-10-2012 23:02, Richard Smith wrote:
>> > 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:
>> >>>>>>>> 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.
>>
>> Ok, but reinitialisation like f = ATOMIC_FLAG_INIT then.
>>
>
> As far as I can see, that is not a valid use of ATOMIC_FLAG_INIT.
>
>
>> While experimenting with this I think I discovered a bug. If you
>> compile the following (essentially atomic_flag initialisation) with
>> clang -S there's no trace of abool_var in the resulting file.
>> ----
>> typedef struct { _Atomic( _Bool ) ab; } abool_t;
>> abool_t abool_var = { 0 };
>> ----
>>
>
> Yikes! Yes, that's completely broken. _Atomic(T) inside structs seems to
> work fine in C++ mode (aside from some conversions causing a crash), but
> does not work at all in C.
>
>
>> In any case you need to use char instead of bool, because the flag can
>> be shared with another piece of hardware that should be allowed to
>> store other values than 1. When testing whether the flag is set the
>> compiler should never assume it can simply compare with 1.
>
>
> Can you cite a source for this claim? C11 7.17.8 says "It has two states,
> set and clear." It seems to me that if you want more than two states and
> you use atomic flag, you're doing it wrong; use an appropriate atomic type
> for the data you are representing.
>
>
>> >> 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.
>>
>> I'm not sure the standard allows you to make the stdint.h and stddef.h
>> symbols visible from stdatomic.h. It would have said so if that were
>> the case, like inttypes.h includes stdint.h, but not stddef.h even
>> though it needs wchar_t for wcstoimax.
>
>
> The burden is on the programmer to not use entities defined in a header
> without including that header. A conforming program cannot tell whether
> standard headers include each other. See 7.1.2/4.
>

Conversely, 7.1.3/1 and 2 says that the identifiers aren't reserved if a
corresponding header isn't included, so I take this back [thanks to
jyasskin for pointing that out!].


> Libc headers define types like
>> __wchar_t for that, e.g. the FreeBSD version of stdatomic.h defines
>> atomic_int_least*_t as _Atomic(__int_least*_t). The compiler headers
>> cannot define such types because that would conflict with system
>> headers
>
>
> Clang provides stdint.h, which defines these types.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121004/1b0e6fe9/attachment.html>