[cfe-commits] C11 <stdatomic.h>

[Tijl Coosemans]

On 04-10-2012 23:04, Richard Smith wrote:
> 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.

I think it's valid, because the other atomic types can be
reinitialised using atomic_init and there's no such function
for atomic_flag.

On the other hand, storing a single byte is always atomic so
atomic_flag_clear_explicit(&f,memory_order_relaxed) is exactly
the same as f = ATOMIC_FLAG_INIT, so you might be right.

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

No, I meant for any non-zero value to be interpreted as set. The
x86 asm for test-and-set is currently something like "movb $1,%al \
xchgb %al,(%ecx) \ andb $1,%al". If you use char instead of bool that
"andb $1,%al" becomes "testb %al,%al" which is less restrictive.

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

Only in the freestanding case.