[cfe-commits] C11 <stdatomic.h>

Tue Oct 9 20:53:37 PDT 2012

On Oct 9, 2012, at 11:17 AM, Jeffrey Yasskin wrote:

> On Tue, Oct 9, 2012 at 10:55 AM, Tijl Coosemans <tijl at coosemans.org> wrote:
>> On 09-10-2012 19:27, Jeffrey Yasskin wrote:
>>> On Tue, Oct 9, 2012 at 9:55 AM, Tijl Coosemans <tijl at coosemans.org> wrote:
>>>> On 08-10-2012 01:34, Jeffrey Yasskin wrote:
>>>>> On Sun, Oct 7, 2012 at 3:42 PM, Tijl Coosemans <tijl at coosemans.org> wrote:
>>>>>> On 07-10-2012 20:53, Richard Smith wrote:
>>>>>>> On Sun, Oct 7, 2012 at 10:53 AM, Tijl Coosemans <tijl at coosemans.org> wrote:
>>>>>>> 
>>>>>>>> On 05-10-2012 20:36, Jeffrey Yasskin wrote:
>>>>>>>>> On Fri, Oct 5, 2012 at 11:27 AM, Tijl Coosemans <tijl at coosemans.org>
>>>>>>>>> wrote:
>>>>>>>>>> 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:
>>>>>>>>>>>>>>> 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.
>>>>>>>>> 
>>>>>>>>> That's a feature request for the C or C++ standard, not something
>>>>>>>>> clang should implement on its own. Remember that Richard is
>>>>>>>>> implementing a spec that's already written, not trying to invent what
>>>>>>>>> might be useful.
>>>>>>>> 
>>>>>>>> Maybe I shouldn't have used the word reinitialisation. It isn't
>>>>>>>> something special. It's what you do when you need to reset some
>>>>>>>> state to recover from an error, e.g. in a device driver if the
>>>>>>>> device crashes you reset the device and reinitialise any state
>>>>>>>> kept by the driver. For normal types you use simple assignment
>>>>>>>> for that, for _Atomic types you can use atomic_init and for
>>>>>>>> atomic_flag (which is not an atomic type) you should be able to
>>>>>>>> assign ATOMIC_FLAG_INIT.
>>>>>>> 
>>>>>>> 'should' here sounds like your own opinion. Can you point to somewhere in
>>>>>>> the C11 standard which justifies this? Why not just use atomic_clear with
>>>>>>> memory_order_relaxed?
>>>>>> 
>>>>>> Well you should be able to do it because there's no alternative.
>>>>>> atomic_clear performs an atomic operation and initialisation
>>>>>> shouldn't require atomicity.
>>>>>> 
>>>>>> Perhaps a better example is:
>>>>>> 
>>>>>> atomic_flag *f = malloc(sizeof(*f));
>>>>>> *f = ATOMIC_FLAG_INIT;
>>>>>> 
>>>>>> If assigning ATOMIC_FLAG_INIT isn't valid you cannot initialise this
>>>>>> flag at all.
>>>>> 
>>>>> 7.17.8 (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf)
>>>>> says, "An atomic_flag that is not explicitly initialized with
>>>>> ATOMIC_FLAG_INIT is initially in an indeterminate state." That is,
>>>>> it's either set or clear, not undefined, so you can put it into a
>>>>> known state by calling atomic_flag_clear().
>>>>> 
>>>>> That does mean that atomic_flag needs to be known to the compiler
>>>>> since it's the only type (or one of very few) that doesn't cause
>>>>> undefined behavior when it's uninitialized.
>>>> 
>>>> Indeterminate means set, clear or trap representation.
>>>> 
>>>> 3.19.2 indeterminate value: either unspecified or a trap representation
>>>> 3.19.3 unspecified value: any valid value may be chosen in every instance
>>>> 3.19.4 trap representation: a value that need not be valid for this type
>>>> 3.19.5 perform a trap: interrupt execution of program such that no further
>>>> operations are performed. Note that fetching a trap representation might
>>>> perform a trap but is not required to.
>>>> 
>>>> So the question is if atomic_flag_clear is guaranteed to work with a
>>>> flag in an invalid state. I think hardware support for this type is
>>>> allowed to assume the flag is in a valid state before any atomic
>>>> operations are used on it. But even if it does work, initialisation
>>>> doesn't require atomicity and shouldn't for performance reasons.
>>> 
>>> Oops. C is different from C++ here, and I didn't double-check before
>>> posting. C++ says, "The macro ATOMIC_FLAG_INIT shall be defined in
>>> such a way that it can be used to initialize an object of type
>>> atomic_flag to the clear state. For a static-duration object, that
>>> initialization shall be static. It is unspecified whether an
>>> unitialized atomic_flag object has an initial state of set or clear."
>>> 
>>> I think you have found a C11 defect here, but again, you should bring
>>> that up with the C committee, not just clang.
>>> 
>>> Note that "performance reasons" are really unconvincing unless you
>>> come with a benchmark.
>> 
>> It seems more like a defect in C++11. C11 had the same wording but
>> they changed it into indeterminate, which makes sense because an
>> uninitialised byte has more values than set and clear. It looks like
>> the C++ committee wanted to adopt this but forgot about it?
>> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1421.pdf item 2.2
>> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1379.htm
> 
> So file it as a C++ defect. In neither case is clang the right place
> to make a decision about it.

FWIW, the committee's intent seems pretty obvious to me here.  They are
declining to specify whether an object with static storage duration
that is uninitialized — that is, which is lacking an explicit initializer and
is therefore implicitly zero-initialized — is in the set or clear state.
They are not trying to require the implementation to guarantee that
default-initialized objects of this type are valid to read from.

>> About the performance reason, I think it's safe to assume that on
>> most if not all architectures non-atomic is faster than atomic.
> 
> It is not safe to assume that. On x86 and ARM, a relaxed atomic store
> is exactly the same instruction as a non-atomic store. The difference
> is in the allowed compiler optimizations, which may or may not apply
> to atomic_flag initialization in actual use.

It is safe to assume that on all architectures, more-constrained
operations are no faster than less-constrained operations.

That said, I agree that an atomic_flag_init function is a feature request
for the standards committees, and that there is no intent that simple
load or assignment of an atomic_flag object has meaningful semantics.

The general presumption of *_INIT macros is that they are not
necessarily legal right operands of assignment operators.  This is
commonplace in POSIX stuff, although I think it may be new to the
C standard.

John.