[cfe-commits] C11 <stdatomic.h>

Mon Sep 22 02:32:58 PDT 2014

+1 on reopening this, 

we currently seem to be non-compliant to c11  on (at least) OSX and Linux (since "the implementation" doesn't provide stdatomic.h, and the compiler doesn't emit __STDC_NO_ATOMICS__) this actually makes it pretty messy for someone to even provide their own local solution (i.e. needs compiler-specific includes).

On 21 Sep 2014, at 14:37, Hal Finkel wrote:

> ----- Original Message -----
>> From: "David Chisnall" <David.Chisnall at cl.cam.ac.uk>
>> To: "Hal Finkel" <hfinkel at anl.gov>
>> Cc: "Richard Smith" <richard at metafoo.co.uk>, "cfe commits" <cfe-commits at cs.uiuc.edu>, "Eli Friedman"
>> <eli.friedman at gmail.com>, "Tijl Coosemans" <tijl at coosemans.org>, "Jeffrey Yasskin" <jyasskin at googlers.com>,
>> "chandlerc" <chandlerc at google.com>, "John McCall" <rjmccall at apple.com>
>> Sent: Sunday, September 21, 2014 5:44:50 AM
>> Subject: Re: [cfe-commits] C11 <stdatomic.h>
>> 
>> We currently have a situation in FreeBSD where several of the
>> clang-provided headers are incompatible with the system headers, and
>> I'd prefer to avoid adding to this.  Our stdatomic.h is carefully
>> designed to work with gcc 4.2.1, clang, and newer GCC, so we
>> definitely wouldn't want to replace it with something
>> clang-specific.  Having clang provide a subset of the C standard
>> headers, independent of the C library, is quite unhelpful to us.
> 
> But gcc does this too, how do you deal with this generally?
> 
> Regardless, we do need a way to deal with the fact that FreeBSD does have a Clang-compatible stdatomic.h (and other systems might too at some point). One potential solution is to structure the header to prefer a system implementation:
> 
> #if __has_include_next(<stdatomic.h>)

I've been using 
#if __STDC_HOSTED__ && __has_include_next(<stdatomic.h>)

locally, modelled on other similar cases.
The stdatomic.h I'm using is otherwise a copy of Richard's from the original thread.

Iain

> # include_next <stdatomic.h>
> #else
> ... (our fall-back implementation)
> #endif
> 
> We might also need to deal with cases where the system does provide a stdatomic.h, but it is not Clang-compatible. To deal with this, we might have the targets with known-Clang-compatible implementations define something (like __CLANG_PREFER_SYSTEM_STDATOMIC_H), and then key off that instead.
> 
> What do you think?
> 
> Thanks again,
> Hal
> 
>> 
>> David
>> 
>> On 21 Sep 2014, at 00:09, Hal Finkel <hfinkel at anl.gov> wrote:
>> 
>>> Richard,
>>> 
>>> I'd like to re-open this issue; our last thread on this seemed to
>>> fizzle out with a discussion on how the ABI should be defined,
>>> Clang/LLVM bugs and probable C11 (or perhaps C++11) defects. This,
>>> however, still leaves our users in the somewhat-unfortunate
>>> situation of not having a stdatomic.h header when using Clang on
>>> Linux (and several other platforms) -- and to make matters worse,
>>> we also don't define __STDC_NO_ATOMICS__. I've received requests
>>> from several of my users to resolve this issue, and I think that
>>> we should move-forward with adding this header. As I'm sure you
>>> know, GCC 4.9 now ships a stdatomic.h, as does FreeBSD. Both seem
>>> to use _Bool as the underlying type for atomic_flag (at least when
>>> __GCC_ATOMIC_TEST_AND_SET_TRUEVAL == 1).
>>> 
>>> Regarding atomic_flag_test_and_set and friends, and whether these
>>> will be defined by libc, regardless of whether we provide proper
>>> definitions, we should follow GCC 4.9 and FreeBSD and define these
>>> functions as macros (this is allowed by 7.1.4). This provides
>>> users with a fully-functional header file regardless of whether
>>> the system libc provides the required external function
>>> implementations (I don't know of any that do).
>>> 
>>> Thanks again,
>>> Hal
>>> 
>>> ----- Original Message -----
>>>> From: "Richard Smith" <richard at metafoo.co.uk>
>>>> To: "Eli Friedman" <eli.friedman at gmail.com>
>>>> Cc: "cfe commits" <cfe-commits at cs.uiuc.edu>
>>>> Sent: Tuesday, September 18, 2012 8:47:37 PM
>>>> Subject: Re: [cfe-commits] C11 <stdatomic.h>
>>>> 
>>>> 
>>>> On Mon, Sep 17, 2012 at 4:11 PM, Eli Friedman <
>>>> eli.friedman at gmail.com > wrote:
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> On Sat, Sep 15, 2012 at 1:05 AM, Richard Smith <
>>>> richard at metafoo.co.uk > wrote:
>>>>> 
>>>>> 
>>>>> On Fri, Sep 14, 2012 at 8:17 PM, Eli Friedman <
>>>>> eli.friedman at gmail.com >
>>>>> wrote:
>>>>>> 
>>>>>> On Fri, Sep 14, 2012 at 7:48 PM, Richard Smith <
>>>>>> richard at metafoo.co.uk >
>>>>>> wrote:
>>>>>>> Hi,
>>>>>>> 
>>>>>>> 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.
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>> 
>>> 
>>> --
>>> Hal Finkel
>>> Assistant Computational Scientist
>>> Leadership Computing Facility
>>> Argonne National Laboratory
>> 
>> 
> 
> -- 
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits