On Fri, Oct 5, 2012 at 11:27 AM, Tijl Coosemans <[email protected]> wrote: > On 04-10-2012 23:04, Richard Smith wrote: >> 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. > > 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. > On the other hand, storing a single byte is always atomic so > atomic_flag_clear_explicit(&f,memory_order_relaxed) is exactly > the same as f = ATOMIC_FLAG_INIT, so you might be right. It's likely that you won't see any difference in performance or possibly even the instructions emitted between atomic_store_explicit(&an_atomic_char, 0, memory_order_relaxed) and atomic_init(&an_atomic_char, 0), yes. The semantics are somewhat different since atomic_init() can cause data races, while atomic_store_explicit cannot. Any requests for new features in this area should come with benchmarks. >>>> 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. > > No, I meant for any non-zero value to be interpreted as set. The > x86 asm for test-and-set is currently something like "movb $1,%al \ > xchgb %al,(%ecx) \ andb $1,%al". If you use char instead of bool that > "andb $1,%al" becomes "testb %al,%al" which is less restrictive. If you want to interact with hardware using C++ atomic types, you're going to need a spec from the hardware maker which clang (and other compilers) could attempt to make implementable. Most likely, the compiler makers would say to implement the hardware's requirements using a non-flag atomic, but we can't be sure without seeing the requirements. Can you link to such a spec, or are you just ... speculating? Jeffrey _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
