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: > >>>>>> On Fri, Sep 14, 2012 at 7:48 PM, Richard Smith < > [email protected] > >>>>>> wrote: > >>>>>>> The attached patch adds an implementation of > >>>>>>> <stdatomic.h> to the set of headers provided by Clang. > >>>>>>> Since this header is so compiler-dependent, it seems that > >>>>>>> we are the most rational component to be providing this > >>>>>>> header (even though, for instance, some flavors of BSD > >>>>>>> already provide their own). > >>>>>>> Please review! > >>>>>> > >>>>>> +// Clang allows memory_order_consume ordering for > __c11_atomic_store, > >>>>>> +// even though C11 doesn't allow it for atomic_store. > >>>>>> > >>>>>> That looks like a bug... > >>>>> > >>>>> Possibly it's a bug in the specification for atomic_flag_clear? > >>>>> memory_order_consume doesn't seem to have any meaning for a store > >>>>> operation. > >>>>> > >>>>>> Please put the new warning in a separate commit. > >>>>> > >>>>> r163964. > >>>>> > >>>>>> 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. > 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.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
