[cfe-commits] C11 <stdatomic.h>

[Jeffrey Yasskin]

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.

Jeffrey