On Thu, Oct 4, 2012 at 12:23 PM, Richard Smith <[email protected]>wrote:
> On Thu, Oct 4, 2012 at 5:18 AM, Tijl Coosemans <[email protected]> wrote: > >> On 03-10-2012 23:02, Richard Smith wrote: >> > On Wed, Oct 3, 2012 at 8:54 AM, Tijl Coosemans <[email protected]> >> wrote: >> >>>> On Tue, Sep 18, 2012 at 6:47 PM, Richard Smith < >> [email protected]> >> >>>> wrote: >> >>>>> On Mon, Sep 17, 2012 at 4:11 PM, Eli Friedman < >> [email protected]> >> >>>>> wrote: >> >>>>>> On Sat, Sep 15, 2012 at 1:05 AM, Richard Smith < >> [email protected]> >> >>>>>> wrote: >> >>>>>>> On Fri, Sep 14, 2012 at 8:17 PM, Eli Friedman >> >>>>>>> <[email protected]> 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. >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
