On Oct 9, 2012, at 11:17 AM, Jeffrey Yasskin wrote: > On Tue, Oct 9, 2012 at 10:55 AM, Tijl Coosemans <[email protected]> wrote: >> On 09-10-2012 19:27, Jeffrey Yasskin wrote: >>> On Tue, Oct 9, 2012 at 9:55 AM, Tijl Coosemans <[email protected]> wrote: >>>> On 08-10-2012 01:34, Jeffrey Yasskin wrote: >>>>> On Sun, Oct 7, 2012 at 3:42 PM, Tijl Coosemans <[email protected]> wrote: >>>>>> On 07-10-2012 20:53, Richard Smith wrote: >>>>>>> On Sun, Oct 7, 2012 at 10:53 AM, Tijl Coosemans <[email protected]> >>>>>>> wrote: >>>>>>> >>>>>>>> On 05-10-2012 20:36, Jeffrey Yasskin wrote: >>>>>>>>> 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: >>>>>>>>>>>>>>> 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. >> >> It seems more like a defect in C++11. C11 had the same wording but >> they changed it into indeterminate, which makes sense because an >> uninitialised byte has more values than set and clear. It looks like >> the C++ committee wanted to adopt this but forgot about it? >> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1421.pdf item 2.2 >> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1379.htm > > So file it as a C++ defect. In neither case is clang the right place > to make a decision about it.
FWIW, the committee's intent seems pretty obvious to me here. They are declining to specify whether an object with static storage duration that is uninitialized — that is, which is lacking an explicit initializer and is therefore implicitly zero-initialized — is in the set or clear state. They are not trying to require the implementation to guarantee that default-initialized objects of this type are valid to read from. >> About the performance reason, I think it's safe to assume that on >> most if not all architectures non-atomic is faster than atomic. > > It is not safe to assume that. On x86 and ARM, a relaxed atomic store > is exactly the same instruction as a non-atomic store. The difference > is in the allowed compiler optimizations, which may or may not apply > to atomic_flag initialization in actual use. It is safe to assume that on all architectures, more-constrained operations are no faster than less-constrained operations. That said, I agree that an atomic_flag_init function is a feature request for the standards committees, and that there is no intent that simple load or assignment of an atomic_flag object has meaningful semantics. The general presumption of *_INIT macros is that they are not necessarily legal right operands of assignment operators. This is commonplace in POSIX stuff, although I think it may be new to the C standard. John. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
