On Thu, Oct 4, 2012 at 5:18 AM, Tijl Coosemans <span dir="ltr"><<a href="mailto:tijl@coosemans.org" target="_blank">tijl@coosemans.org</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On 03-10-2012 23:02, Richard Smith wrote:<br>
> On Wed, Oct 3, 2012 at 8:54 AM, Tijl Coosemans <<a href="mailto:tijl@coosemans.org">tijl@coosemans.org</a>> wrote:<br>
>>>> On Tue, Sep 18, 2012 at 6:47 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>
>>>> wrote:<br>
>>>>> On Mon, Sep 17, 2012 at 4:11 PM, Eli Friedman <<a href="mailto:eli.friedman@gmail.com">eli.friedman@gmail.com</a>><br>
>>>>> wrote:<br>
>>>>>> On Sat, Sep 15, 2012 at 1:05 AM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>
>>>>>> wrote:<br>
>>>>>>> On Fri, Sep 14, 2012 at 8:17 PM, Eli Friedman<br>
>>>>>>> <<a href="mailto:eli.friedman@gmail.com">eli.friedman@gmail.com</a>> wrote:<br>
</div><div><div class="h5">>>>>>>>> It looks like standard requires that we expose functions named<br>
>>>>>>>> atomic_thread_fence, atomic_signal_fence, atomic_flag_test_and_set,<br>
>>>>>>>> atomic_flag_test_and_set_explicit, and atomic_flag_clear; your<br>
>>>>>>>> version of stdatomic.h doesn't include declarations for these<br>
>>>>>>>> functions (which is required by C11 7.1.4p1).<br>
>>>>>>><br>
>>>>>>> Ugh. And C11 7.1.2/6 requires them to have external linkage.<br>
>>>>>>> I don't want these functions to require linking to a library.<br>
>>>>>>> We could emit them weak and inline, but then we'll get a weak<br>
>>>>>>> copy in every TU which includes this header, which seems<br>
>>>>>>> fairly egregious. Is there currently any way to emit a<br>
>>>>>>> function as linkonce_odr from C? Do you have any suggestions<br>
>>>>>>> as to how to proceed?<br>
>>>>>><br>
>>>>>> There isn't any way to get linkonce_odr from C at the moment; patches<br>
>>>>>> welcome.  I don't see any issues with that from the standpoint of the<br>
>>>>>> standard; I'm a little worried about ABI-compat issues, though.<br>
>>>>>> (Specifically, if the system provides the header, having our own<br>
>>>>>> linkonce_odr version could cause strange linker errors.)<br>
>>>>>><br>
>>>>>> We could put it into compiler-rt, and say that if someone tries to use<br>
>>>>>> the function instead of the macro without linking in compiler-rt,<br>
>>>>>> that's an error.  Not particularly satisfying either, but somewhat<br>
>>>>>> simpler.<br>
>>>>><br>
>>>>> After some discussion with Chandler, we think the best approach is to<br>
>>>>> say that the definition of these functions belongs in libc, and to<br>
>>>>> provide only declarations of them. A patch for that approach is<br>
>>>>> attached.<br>
>><br>
>> If the compiler provides stdatomic.h but libc implements the functions<br>
>> then all compilers need to have a binary compatible definition of<br>
>> memory_order and atomic_flag types.<br>
><br>
> [On the assumption that these atomic operations, and stdatomic.h itself,<br>
> should be provided by the compiler rather than by libc...] The definition<br>
> of atomic_flag needs to be part of the ABI anyway, if we want to have any<br>
> hope of compiler interoperability with (structs containing) atomic types,<br>
> so I view that part as a non-issue. The memory_order part is unfortunate,<br>
> though a libc could choose to ignore that parameter and always treat it as<br>
> seq_cst (since this only affects situations where the macro definition<br>
> isn't being used, it should be very rare).<br>
><br>
>> The patch implements atomic_flag on top of atomic_bool, but that means<br>
>> atomic_flag f = ATOMIC_FLAG_INIT is an atomic store.<br>
><br>
> Not true. There is no need for the initialization of an _Atomic variable to<br>
> use an atomic write, and the code Clang emits does not perform one.<br>
<br>
</div></div>Ok, but reinitialisation like f = ATOMIC_FLAG_INIT then.<br></blockquote><div><br></div><div>As far as I can see, that is not a valid use of ATOMIC_FLAG_INIT.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

While experimenting with this I think I discovered a bug. If you<br>
compile the following (essentially atomic_flag initialisation) with<br>
clang -S there's no trace of abool_var in the resulting file.<br>
----<br>
typedef struct { _Atomic( _Bool ) ab; } abool_t;<br>
abool_t abool_var = { 0 };<br>
----<br></blockquote><div><br></div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
In any case you need to use char instead of bool, because the flag can<br>
be shared with another piece of hardware that should be allowed to<br>
store other values than 1. When testing whether the flag is set the<br>
compiler should never assume it can simply compare with 1.</blockquote><div><br></div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
>> Also instead of including stdint.h and stddef.h, maybe you can use<br>
>> preprocessor macros like __INT8_TYPE__.<br>
><br>
> Much of the work in stdint.h goes into defining the int_least*_t and<br>
> int_fast*_t types correctly, and it doesn't make sense to duplicate that<br>
> when defining atomic_int_least*_t etc. I don't think removing these<br>
> dependencies is particularly worthwhile.<br>
<br>
</div>I'm not sure the standard allows you to make the stdint.h and stddef.h<br>
symbols visible from stdatomic.h. It would have said so if that were<br>
the case, like inttypes.h includes stdint.h, but not stddef.h even<br>
though it needs wchar_t for wcstoimax.</blockquote><div><br></div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Libc headers define types like<br>
__wchar_t for that, e.g. the FreeBSD version of stdatomic.h defines<br>
atomic_int_least*_t as _Atomic(__int_least*_t). The compiler headers<br>
cannot define such types because that would conflict with system<br>
headers</blockquote><div><br></div><div>Clang provides stdint.h, which defines these types.</div></div>