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. Jeffrey _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
