RFC: __atomic_* support for gcc 4.7 compatibility

classic Classic list List threaded Threaded
27 messages Options
12
Reply | Threaded
Open this post in threaded view
|

RFC: __atomic_* support for gcc 4.7 compatibility

Richard Smith
Hi,

I'm looking into adding support for gcc 4.7's __atomic_* builtins for the clang 3.1 release (these are necessary to support various parts of libstdc++4.7). Our current set of __atomic_* builtins is incompatible with gcc 4.7's builtins in a few ways:

1) In clang, the builtins produce results by value, whereas in gcc, a pointer is passed indicating where the result should be written.
2) In clang, the argument type must be an _Atomic. gcc does not have this restriction (but also fails to require that the type is trivially copyable, which is probably a bug).
3) Clang is missing the __atomic_*_n, __atomic_always_lock_free and __atomic_fetch_nand builtins.
4) Clang is missing the __atomic_compare_exchange builtin, and instead provides __atomic_compare_exchange_strong and __atomic_compare_exchange_weak.
5) Clang provides an __atomic_init builtin, which gcc does not provide, and which crashes during CodeGen.

My current plan is:

For (1), add support for the gcc-style arguments, then port libc++ over to them, then remove support for the current arguments.
For (2), relax the requirement to the type being either _Atomic or trivially copyable.
For (3) and (4), add the missing builtins. I don't intend to remove the __atomic_compare_exchange_strong/__atomic_compare_exchange_weak builtins, but will do so if anyone particularly wants to see them gone.
For (5), remove this builtin.

Please shout now if any part of this displeases you! In particular, if you have code depending on clang's current pass-by-value atomics and you'd like to see them left in until after the clang 3.1 release to give you more time to move to the portable forms (or indeed you believe they should be left in-place forever), please say so.

Thanks!
Richard

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: __atomic_* support for gcc 4.7 compatibility

Richard Smith
On Tue, Apr 10, 2012 at 12:22 PM, Richard Smith <[hidden email]> wrote:
5) Clang provides an __atomic_init builtin, which gcc does not provide, and which crashes during CodeGen.

I should clarify: this builtin works fine on scalar types, but crashes in cases like this:

  struct T { char v[4]; };
  _Atomic(T) a;
  void f() { __atomic_init(&a, T()); }

If this builtin is useful (in particular for C11, since neither libstdc++ nor libc++ need it), I'll take a look at fixing it instead of removing it.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: __atomic_* support for gcc 4.7 compatibility

David Chisnall
In reply to this post by Richard Smith
On 10 Apr 2012, at 20:22, Richard Smith wrote:

> Hi,
>
> I'm looking into adding support for gcc 4.7's __atomic_* builtins for the clang 3.1 release (these are necessary to support various parts of libstdc++4.7). Our current set of __atomic_* builtins is incompatible with gcc 4.7's builtins in a few ways:

While it would be nice to build libstdc++, it should be noted that the gcc builtins are poorly designed in a number of ways and I don't think we should be encouraging their use by supporting them in anything other than the specific case of supporting libstdc++.   The clang ones are a closer match for the C11 spec (and we should really be encouraging people to use stdatomic.h, rather than using the intrinsics directly anyway).

> 1) In clang, the builtins produce results by value, whereas in gcc, a pointer is passed indicating where the result should be written.

This means that the things required by C11's stdatomic.h can be trivially implemented as macros (just dropping the __ prefix) with the clang builtins.  They can not with the GCC ones in their standard form, although the _n versions can be used this way.  I am not sure what the perceived advantage of the GCC non-_n ones is.  

> 2) In clang, the argument type must be an _Atomic. gcc does not have this restriction (but also fails to require that the type is trivially copyable, which is probably a bug).

This is a horrible bug in GCC, because it means that you can have atomic and non-atomic accesses to the same memory address (something explicitly not allowed by C11).  This is part of the reason for the _Atomic() qualifier.  It also changes the alignment requirements of certain types, in both gcc and clang, which can lead to really hard-to-track-down bugs as a result of sloppy casting.  

I would strongly oppose supporting using the builtins with types that are not _Atomic-qualified, because doing so makes it trivial to introduce a large number of very subtle bugs.

> 3) Clang is missing the __atomic_*_n, __atomic_always_lock_free and __atomic_fetch_nand builtins.

The _n builtins actually do what the current clang ones do.  __atomic_always_lock_free is probably useful.  __atomic_fetch_nand might be as well, although it isn't needed by C++11 or C11.

> 4) Clang is missing the __atomic_compare_exchange builtin, and instead provides __atomic_compare_exchange_strong and __atomic_compare_exchange_weak.

I'm not sure what the point of the GCC version is.  The strong / weak option affects code generation, it's not something that can be selected at run time, so having a bool parameter just makes code less readable.

> 5) Clang provides an __atomic_init builtin, which gcc does not provide, and which crashes during CodeGen.

This is required for C11.  I'll take a look at the crash.  It's also used by libc++.  __atomic_init() initialises an _Atomic type without performing an atomic operation (as per both C++11 and C11).  Or, at least, it should...

I believe that libstdc++ gets around this by not _Atomic-qualifying the types and then using normal initialisation.  This is... problematic, to say the least, as an approach.  

> My current plan is:
>
> For (1), add support for the gcc-style arguments, then port libc++ over to them, then remove support for the current arguments.

Please don't.  

> For (2), relax the requirement to the type being either _Atomic or trivially copyable.

I think this would be a serious mistake.  Allowing atomic operations on non-_Atomic types in the intrinsics makes it insanely hard to correctly implement C11 stdatomic.h and will cause all sorts of problems for users of these builtins later.

> For (3) and (4), add the missing builtins. I don't intend to remove the __atomic_compare_exchange_strong/__atomic_compare_exchange_weak builtins, but will do so if anyone particularly wants to see them gone.

As with the other clang builtins, they make stdatomic.h much easier to implement.  From FreeBSD's stdatomic.h:

#if defined(__CLANG_ATOMICS)
#define atomic_compare_exchange_strong_explicit(object, expected,       \
    desired, success, failure)                                          \
        __atomic_compare_exchange_strong(object, expected, desired,     \
            success, failure)
#define atomic_compare_exchange_weak_explicit(object, expected,         \
    desired, success, failure)                                          \
        __atomic_compare_exchange_weak(object, expected, desired,       \
            success, failure)

Replacing these with a bool parameter would probably be okay, but it makes the code less readable.  Which of these do you find easier to understand?

__atomic_compare_exchange_strong(&a, &new, 12, __ATOMIC_SEQ_CST, __ATOMIC_RELAXED);

Or:

__atomic_compare_exchange_strong(&a, &new, 12, true, __ATOMIC_SEQ_CST, __ATOMIC_RELAXED);

What is that true for?  What happens if it's a variable?  I honestly have no idea - switching between strong and weak implementations at run time is more or less impossible.

> For (5), remove this builtin.

Again, please don't, it is required for C11 and makes achieving the correct semantics for C++11 much easier.

> Please shout now if any part of this displeases you! In particular, if you have code depending on clang's current pass-by-value atomics and you'd like to see them left in until after the clang 3.1 release to give you more time to move to the portable forms (or indeed you believe they should be left in-place forever), please say so.

See above.

David

-- Sent from my Cray X1


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: __atomic_* support for gcc 4.7 compatibility

Richard Smith
On Tue, Apr 10, 2012 at 12:58 PM, David Chisnall <[hidden email]> wrote:
On 10 Apr 2012, at 20:22, Richard Smith wrote:
> Hi,
>
> I'm looking into adding support for gcc 4.7's __atomic_* builtins for the clang 3.1 release (these are necessary to support various parts of libstdc++4.7). Our current set of __atomic_* builtins is incompatible with gcc 4.7's builtins in a few ways:

While it would be nice to build libstdc++, it should be noted that the gcc builtins are poorly designed in a number of ways and I don't think we should be encouraging their use by supporting them in anything other than the specific case of supporting libstdc++.

I agree, but I think this is a problem of documentation. On that subject, do we have documentation for our current __atomic_ builtins somewhere?

> 1) In clang, the builtins produce results by value, whereas in gcc, a pointer is passed indicating where the result should be written.

This means that the things required by C11's stdatomic.h can be trivially implemented as macros (just dropping the __ prefix) with the clang builtins.  They can not with the GCC ones in their standard form, although the _n versions can be used this way.  I am not sure what the perceived advantage of the GCC non-_n ones is.

> 2) In clang, the argument type must be an _Atomic. gcc does not have this restriction (but also fails to require that the type is trivially copyable, which is probably a bug).

This is a horrible bug in GCC, because it means that you can have atomic and non-atomic accesses to the same memory address (something explicitly not allowed by C11).  This is part of the reason for the _Atomic() qualifier.  It also changes the alignment requirements of certain types, in both gcc and clang, which can lead to really hard-to-track-down bugs as a result of sloppy casting.

I don't see that this is obviously a bug in gcc -- it does not support _Atomic, and (as you suggested above) presumably expects its builtins to be used only to implement <stdatomic.h> and <atomic> (or by people who know what they're doing and want to circumvent the normal protections).

I would strongly oppose supporting using the builtins with types that are not _Atomic-qualified, because doing so makes it trivial to introduce a large number of very subtle bugs.

Such support is necessary to support libstdc++'s <atomic>. I think you've made a reasonable case that we should have two sets of builtins, a C11-style set (the current builtins) and a GCC-compatible set. To this end, I would strongly prefer for them to have different names, so that the trivial mapping of <stdatomic.h> names to builtins never accidentally uses the GCC-compatible builtins. Unfortunately, that seems to require renaming the C11-style builtins (unless someone has a better suggestion).
 
> 3) Clang is missing the __atomic_*_n, __atomic_always_lock_free and __atomic_fetch_nand builtins.

The _n builtins actually do what the current clang ones do.

Functionally, yes, but the _Atomic check makes clang's implementations unusable for libstdc++, and the _n variants are required to check that the argument type is a 1, 2, 4, 8 or 16-byte integer or pointer type.

__atomic_always_lock_free is probably useful.  __atomic_fetch_nand might be as well, although it isn't needed by C++11 or C11.

Right. I consider __atomic_fetch_nand and __atomic_always_lock_free to be a low priority (not necessary for 3.1) since libstdc++4.7 does not use them.

> 4) Clang is missing the __atomic_compare_exchange builtin, and instead provides __atomic_compare_exchange_strong and __atomic_compare_exchange_weak.

I'm not sure what the point of the GCC version is.  The strong / weak option affects code generation, it's not something that can be selected at run time, so having a bool parameter just makes code less readable.

GCC's (documented) approach is to use the strong version if it cannot prove the 'weak' bool is true. I agree that this seems like a strange choice (and indeed in libstdc++ the 'weak' flag is always one of 0, 1, false, or true), but it's necessary for libstdc++ compatibility, so if we want such compatibility, our aesthetic preferences are somewhat irrelevant.

The same objection also applies to the memory order parameter (where GCC always uses seq_cst if it can't determine the argument at build-time, and clang generates a branch).

> 5) Clang provides an __atomic_init builtin, which gcc does not provide, and which crashes during CodeGen.

This is required for C11.  I'll take a look at the crash.  It's also used by libc++.  __atomic_init() initialises an _Atomic type without performing an atomic operation (as per both C++11 and C11).  Or, at least, it should...

I believe that libstdc++ gets around this by not _Atomic-qualifying the types and then using normal initialisation.  This is... problematic, to say the least, as an approach.

libc++ does not currently use __atomic_init; it uses a sequentially-consistent __atomic_store instead. I agree that that seems highly suboptimal, and that we should keep this builtin.
 
> My current plan is:
>
> For (1), add support for the gcc-style arguments, then port libc++ over to them, then remove support for the current arguments.

Please don't.

Your objections thus far have not been strong enough, in my opinion, to justify abandoning libstdc++4.7 support for clang, nor to justify abandoning g++ support for libc++ (although naturally that will be Howard's choice). I'm not opposed to keeping the existing builtins too, as distasteful as it is to have two competing, and very similar, sets.

> For (2), relax the requirement to the type being either _Atomic or trivially copyable.

I think this would be a serious mistake.  Allowing atomic operations on non-_Atomic types in the intrinsics makes it insanely hard to correctly implement C11 stdatomic.h and will cause all sorts of problems for users of these builtins later.

Given the above approach to (1), I think the best plan here would be to leave the C11 intrinsics alone and require the argument for the GNU intrinsics to be trivially-copyable. Thus the GNU intrinsics would not work with _Atomic types, and the _Atomic intrinsics would not work with non-_Atomic types.

> For (3) and (4), add the missing builtins. I don't intend to remove the __atomic_compare_exchange_strong/__atomic_compare_exchange_weak builtins, but will do so if anyone particularly wants to see them gone.

As with the other clang builtins, they make stdatomic.h much easier to implement.
[...]

As I said, I don't intend to remove the strong/weak variants.
 
> For (5), remove this builtin.

Again, please don't, it is required for C11 and makes achieving the correct semantics for C++11 much easier.

Agreed.

Many thanks for your feedback!
Richard

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: __atomic_* support for gcc 4.7 compatibility

David Chisnall-2

On 10 Apr 2012, at 22:11, Richard Smith wrote:

> On Tue, Apr 10, 2012 at 12:58 PM, David Chisnall <[hidden email]> wrote:
> On 10 Apr 2012, at 20:22, Richard Smith wrote:
> > Hi,
> >
> > I'm looking into adding support for gcc 4.7's __atomic_* builtins for the clang 3.1 release (these are necessary to support various parts of libstdc++4.7). Our current set of __atomic_* builtins is incompatible with gcc 4.7's builtins in a few ways:
>
> While it would be nice to build libstdc++, it should be noted that the gcc builtins are poorly designed in a number of ways and I don't think we should be encouraging their use by supporting them in anything other than the specific case of supporting libstdc++.
>
> I agree, but I think this is a problem of documentation. On that subject, do we have documentation for our current __atomic_ builtins somewhere?

The documentation is the C11 spec.  The builtins have a 1:1 correspondence with the C11 generic functions in stdatomic.h.

> > 2) In clang, the argument type must be an _Atomic. gcc does not have this restriction (but also fails to require that the type is trivially copyable, which is probably a bug).
>
> This is a horrible bug in GCC, because it means that you can have atomic and non-atomic accesses to the same memory address (something explicitly not allowed by C11).  This is part of the reason for the _Atomic() qualifier.  It also changes the alignment requirements of certain types, in both gcc and clang, which can lead to really hard-to-track-down bugs as a result of sloppy casting.
>
> I don't see that this is obviously a bug in gcc -- it does not support _Atomic, and (as you suggested above)

In that case, it is an additional bug, because its documentation on its atomic implementation indicates that it does.

> presumably expects its builtins to be used only to implement <stdatomic.h> and <atomic> (or by people who know what they're doing and want to circumvent the normal protections).

As I said, it's very difficult to enforce the requirement that _Atomic operations only be accessed atomically in C (not so hard in C++, where you have operator overloading).  For example, in C11 this:

_Atomic(int) i;
...
i++;

Is an atomic increment.  And so is this:

atomic_fetch_add_explicit(&i, 1, memory_order_seq_cst);

But this i++ is not atomic if i is not _Atomic-qualified, and atomic_fetch_add_explicit() is undefined in this case.

> I would strongly oppose supporting using the builtins with types that are not _Atomic-qualified, because doing so makes it trivial to introduce a large number of very subtle bugs.
>
> Such support is necessary to support libstdc++'s <atomic>. I think you've made a reasonable case that we should have two sets of builtins, a C11-style set (the current builtins) and a GCC-compatible set. To this end, I would strongly prefer for them to have different names, so that the trivial mapping of <stdatomic.h> names to builtins never accidentally uses the GCC-compatible builtins. Unfortunately, that seems to require renaming the C11-style builtins (unless someone has a better suggestion).

I believe that it's possible to implement the GCC ones in a terms of the clang ones in a header, if they're required for libstdc++.  If libstdc++ is the only consumer of them, then an <atomic> that did this and then #include_next'd the libstdc++ one might be a better solution.

> > 3) Clang is missing the __atomic_*_n, __atomic_always_lock_free and __atomic_fetch_nand builtins.
>
> The _n builtins actually do what the current clang ones do.
>
> Functionally, yes, but the _Atomic check makes clang's implementations unusable for libstdc++, and the _n variants are
> required to check that the argument type is a 1, 2, 4, 8 or 16-byte integer or pointer type.

The latter requirement is not required by either C++11 or C11.

> __atomic_always_lock_free is probably useful.  __atomic_fetch_nand might be as well, although it isn't needed by C++11 or C11.
>
> Right. I consider __atomic_fetch_nand and __atomic_always_lock_free to be a low priority (not necessary for 3.1) since libstdc++4.7 does not use them.
>
> > 4) Clang is missing the __atomic_compare_exchange builtin, and instead provides __atomic_compare_exchange_strong and __atomic_compare_exchange_weak.
>
> I'm not sure what the point of the GCC version is.  The strong / weak option affects code generation, it's not something that can be selected at run time, so having a bool parameter just makes code less readable.
>
> GCC's (documented) approach is to use the strong version if it cannot prove the 'weak' bool is true. I agree that this seems like a strange choice (and indeed in libstdc++ the 'weak' flag is always one of 0, 1, false, or true), but it's necessary for libstdc++ compatibility, so if we want such compatibility, our aesthetic preferences are somewhat irrelevant.

#define __atomic_compare_exchage(a,b,c,d,e) __atomic_compare_exchange_strong(a,b,d,e)

> The same objection also applies to the memory order parameter (where GCC always uses seq_cst if it can't determine the argument at build-time, and clang generates a branch).

The difference being that the C11 and C++11 specs make the memory order parameters explicit, but they make the strong / weak version part of the operation name.

> > 5) Clang provides an __atomic_init builtin, which gcc does not provide, and which crashes during CodeGen.
>
> This is required for C11.  I'll take a look at the crash.  It's also used by libc++.  __atomic_init() initialises an _Atomic type without performing an atomic operation (as per both C++11 and C11).  Or, at least, it should...
>
> I believe that libstdc++ gets around this by not _Atomic-qualifying the types and then using normal initialisation.  This is... problematic, to say the least, as an approach.
>
> libc++ does not currently use __atomic_init; it uses a sequentially-consistent __atomic_store instead.

That's a bug then.  The C++11 spec explicitly states that initialisation is NOT atomic (as Howard pointed out to me when I tried to introduce this bug in libc++).

> I agree that that seems highly suboptimal, and that we should keep this builtin.
> > My current plan is:
> >
> > For (1), add support for the gcc-style arguments, then port libc++ over to them, then remove support for the current arguments.
>
> Please don't.
>
> Your objections thus far have not been strong enough, in my opinion, to justify abandoning libstdc++4.7 support for clang, nor to justify abandoning g++ support for libc++ (although naturally that will be Howard's choice). I'm not opposed to keeping the existing builtins too, as distasteful as it is to have two competing, and very similar, sets.

I'm not opposing supporting libstdc++, but I am not convinced that introducing a set of poorly designed intrinsics is the correct way of doing it, especially since we would be forced to support them long-term.

-- Sent from my Cray X1


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: __atomic_* support for gcc 4.7 compatibility

Howard Hinnant
It looks like we have a couple of questions:

1.  Does clang support libstdc++ <atomic>?

2.  Does libc++ <atomic> support gcc?

I have no opinion on 1.  I will leave that to the clang developers.

On 2, I have no motivation to support gcc.  gcc has its own std::lib solution and the GPL is not compatible with libc++ licensing.  However if there are members of this community that wish to support gcc (and wish to do the work to make it so), and if there is zero run time and code size overhead, and if the compile time overhead is negligible, I have no objection.  I will cringe at the ugliness it will introduce to the source, but I can live with that.

Since I do not care about gcc compatibility, I prefer whatever intrinsics have the most seamless matching to C11/C++11.  I'm under the impression that this is the current clang design, but I am not expert in this area.

I do not understand the need for __atomic_init.  The atomic constructors that currently look like:

    /*constexpr*/ __atomic_base(_Tp __d) { __atomic_store(&__a_, __d, memory_order_seq_cst); }

used to look like:

   /*constexpr*/ __atomic_base(_Tp __d) : __a_(__d) {}

and it is my understanding that the latter is correct.

If the latter has the correct semantics, I prefer it to:

   /*constexpr*/ __atomic_base(_Tp __d)  {__atomic_init(&__a_, __d);}

An intrinsic that does nothing but initialize the same way a C/C++ statement would do, adds no value, but does add confusion over what it is doing.  If __atomic_init does add needed functionality, then of course I would agree that we need it.

David, could you revert the initialization statements to use either plain C++, or the __atomic_init intrinsic?

<atomic> also needs to be treated with constexpr and noexcept, although there is no rush to do that prior to having the intrinsics fully operational (or are they already?).

Thanks,
Howard

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: __atomic_* support for gcc 4.7 compatibility

David Chisnall-2
On 10 Apr 2012, at 22:57, Howard Hinnant wrote:

> David, could you revert the initialization statements to use either plain C++, or the __atomic_init intrinsic?

Sorry, I thought I'd done that already when I fixed initialisation of _Atomic types in clang.  I'm just about to fall asleep, but I'll do it in the morning.  

__atomic_init() is still required for atomic_init() (in both C11 and C++11) - these are currently using __atomic_store, in libc++, so I'll fix them at the same time.

David


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: __atomic_* support for gcc 4.7 compatibility

Howard Hinnant
On Apr 10, 2012, at 6:11 PM, David Chisnall wrote:

> On 10 Apr 2012, at 22:57, Howard Hinnant wrote:
>
>> David, could you revert the initialization statements to use either plain C++, or the __atomic_init intrinsic?
>
> Sorry, I thought I'd done that already when I fixed initialisation of _Atomic types in clang.  I'm just about to fall asleep, but I'll do it in the morning.  

Of course.  I'd much rather you be awake when you do this. :-)

>
> __atomic_init() is still required for atomic_init() (in both C11 and C++11) - these are currently using __atomic_store, in libc++, so I'll fix them at the same time.

template <class _Tp>
inline _LIBCPP_INLINE_VISIBILITY
void
atomic_init(atomic<_Tp>* __o, _Tp __d)
{
    __o->__a_ = __d;
}

Not right?

Howard

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: __atomic_* support for gcc 4.7 compatibility

Richard Smith
In reply to this post by David Chisnall-2
On Tue, Apr 10, 2012 at 2:25 PM, David Chisnall <[hidden email]> wrote:
On 10 Apr 2012, at 22:11, Richard Smith wrote:
 > On Tue, Apr 10, 2012 at 12:58 PM, David Chisnall <[hidden email]> wrote:
> On 10 Apr 2012, at 20:22, Richard Smith wrote:
> > Hi,
> >
> > I'm looking into adding support for gcc 4.7's __atomic_* builtins for the clang 3.1 release (these are necessary to support various parts of libstdc++4.7). Our current set of __atomic_* builtins is incompatible with gcc 4.7's builtins in a few ways:
>
> While it would be nice to build libstdc++, it should be noted that the gcc builtins are poorly designed in a number of ways and I don't think we should be encouraging their use by supporting them in anything other than the specific case of supporting libstdc++.
>
> I agree, but I think this is a problem of documentation. On that subject, do we have documentation for our current __atomic_ builtins somewhere?

The documentation is the C11 spec.  The builtins have a 1:1 correspondence with the C11 generic functions in stdatomic.h.

The C11 spec does not talk about __atomic_*. A statement in our documentation saying that these builtins implement the <stdatomic.h> operations of the same name would be useful (especially since our builtins have the same name as, but different behaviour from, the GCC builtins).

> I would strongly oppose supporting using the builtins with types that are not _Atomic-qualified, because doing so makes it trivial to introduce a large number of very subtle bugs.
>
> Such support is necessary to support libstdc++'s <atomic>. I think you've made a reasonable case that we should have two sets of builtins, a C11-style set (the current builtins) and a GCC-compatible set. To this end, I would strongly prefer for them to have different names, so that the trivial mapping of <stdatomic.h> names to builtins never accidentally uses the GCC-compatible builtins. Unfortunately, that seems to require renaming the C11-style builtins (unless someone has a better suggestion).

I believe that it's possible to implement the GCC ones in a terms of the clang ones in a header, if they're required for libstdc++.  If libstdc++ is the only consumer of them, then an <atomic> that did this and then #include_next'd the libstdc++ one might be a better solution.

I assume you're suggesting that we cast the argument from T* to _Atomic(T*) prior to passing it into the builtin? That's ugly, but seems like it might be workable if _Atomic(T) always has the same size and alignment as T. I don't know if those properties hold, but I understand the intent of _Atomic() was that they need not. (We would also need to ensure TBAA knows that T and _Atomic(T) can alias.)

This approach, including the need to interpose in front of <atomic> and <shared_ptr> (and any other user of the atomics builtins in libstdc++) and detect whether we're about to include a header from libstdc++, seems sufficiently fragile that I'm still in favour of introducing GNU-compatible builtins.

> > 3) Clang is missing the __atomic_*_n, __atomic_always_lock_free and __atomic_fetch_nand builtins.
>
> The _n builtins actually do what the current clang ones do.
>
> Functionally, yes, but the _Atomic check makes clang's implementations unusable for libstdc++, and the _n variants are
> required to check that the argument type is a 1, 2, 4, 8 or 16-byte integer or pointer type.

The latter requirement is not required by either C++11 or C11.

If we want to fully support the GNU-style atomics, we should implement them correctly. (Naturally, if we're using your compatibility-shim approach, we needn't worry about this.)
 
> > My current plan is:
> >
> > For (1), add support for the gcc-style arguments, then port libc++ over to them, then remove support for the current arguments.
>
> Please don't.
>
> Your objections thus far have not been strong enough, in my opinion, to justify abandoning libstdc++4.7 support for clang, nor to justify abandoning g++ support for libc++ (although naturally that will be Howard's choice). I'm not opposed to keeping the existing builtins too, as distasteful as it is to have two competing, and very similar, sets.

I'm not opposing supporting libstdc++, but I am not convinced that introducing a set of poorly designed intrinsics is the correct way of doing it, especially since we would be forced to support them long-term.

I share your concern. On the other hand, we have a long history of supporting gcc features, even when we think their design is questionable, for compatibility.

- Richard

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: __atomic_* support for gcc 4.7 compatibility

Chandler Carruth
In reply to this post by Howard Hinnant
To comment first on your high level questions, because I think it's important to quickly get some consensus here.

On Tue, Apr 10, 2012 at 11:57 PM, Howard Hinnant <[hidden email]> wrote:
It looks like we have a couple of questions:

1.  Does clang support libstdc++ <atomic>?

2.  Does libc++ <atomic> support gcc?

I have no opinion on 1.  I will leave that to the clang developers.

For us, this is a simple necessity. We cannot use Clang without also using libstdc++ at least for the time being. I don't claim that this is a perfect state of the world, but it is a strongly desirable one.

One perhaps important point -- the libstdc++ developers have thus far been perfectly receptive of C++ conformance patches only required for Clang. While it seems hopeless to get them to not support GCC builtins, they are meeting us part of the way there.

Also note that we have a long history in Clang of supporting GCC builtins to ease migration. I see the atomics in a very similar light.
 
On 2, I have no motivation to support gcc.  gcc has its own std::lib solution and the GPL is not compatible with libc++ licensing.  However if there are members of this community that wish to support gcc (and wish to do the work to make it so), and if there is zero run time and code size overhead, and if the compile time overhead is negligible, I have no objection.  I will cringe at the ugliness it will introduce to the source, but I can live with that.

I would like to suggest that we aim for this as well. Certainly for us, it helps to have both standard libraries and compilers be cross-tested with each other. This makes evaluating them independently and supporting them in different contexts easier and more predictable.

I generally think that as the less-adopted system, it would benefit libc++ (similarly to the way it benefits Clang) to be as compatible as it is reasonable to be.

That said, I do firmly agree with your restrictions: it should never make using libc++ with Clang *worse* only to get support for GCC. Users should not pay for what they do not need.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: __atomic_* support for gcc 4.7 compatibility

Chandler Carruth
In reply to this post by Richard Smith
I want to call out this point:

On Wed, Apr 11, 2012 at 12:25 AM, Richard Smith <[hidden email]> wrote:
This approach, including the need to interpose in front of <atomic> and <shared_ptr> (and any other user of the atomics builtins in libstdc++) and detect whether we're about to include a header from libstdc++, seems sufficiently fragile that I'm still in favour of introducing GNU-compatible builtins.

I think this is setting ourselves up for an endless game of catchup. What is worse, our users will suffer.

Version skew between libstdc++ and Clang is inevitable. As a consequence, the advantages of being maximally compatible with libstdc++ are only present when we can be largely *forward* compatible. This seems impossible if we have to intercept ever header to reference a GCC builtin and introduce our wrapper for it.


At a fundamental level, I am not yet comfortable with Clang, after defining __GNUC__ and acting as a heavily compatible compiler with GCC, going on to define builtin functions which share the same name but incompatible semantics with GCC builtins. This seems like a recipe for a long string of puzzling compilation failures, user frustration, and impeded adoption of Clang.

I think we should continue with the long standing policy of, when reasonable and tenable, supporting the GCC builtins with their names, and allowing users to not care. Then, under a *different* naming pattern, and guarded by proper __has_feature macros etc, we should provide Clang-specific extensions which have the semantics we would rather see.

While a realize this isn't ideal, and it requires renaming currently supported intrinsics I see few options left to us:
1) We haven't released with these intrinsics
2) GCC has released with its intrinsics
3) We failed to discuss our intrinsics sufficiently with the GCC folks to get them to implement the same set
4) GCC folks failed to discuss theirs with us sufficiently to get us to implement the same set
5) We need something for Clang v3.1, or it is instantly un-usable with GCC 4.7 and later.

I don't like any of this, and it isn't how I would wish for things to proceed if I had it to do again, but this is where we are. We need to realize that, today, I cannot compile any meaningful C++98 code with the default C++ standard library on Linux (after a distro picks up GCC 4.7) and Clang. I do not think that is an acceptable state to release Clang under.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: __atomic_* support for gcc 4.7 compatibility

Howard Hinnant
On Apr 10, 2012, at 7:24 PM, Chandler Carruth wrote:

> I want to call out this point:
>
> On Wed, Apr 11, 2012 at 12:25 AM, Richard Smith <[hidden email]> wrote:
> This approach, including the need to interpose in front of <atomic> and <shared_ptr> (and any other user of the atomics builtins in libstdc++) and detect whether we're about to include a header from libstdc++, seems sufficiently fragile that I'm still in favour of introducing GNU-compatible builtins.
>
> I think this is setting ourselves up for an endless game of catchup. What is worse, our users will suffer.
>
> Version skew between libstdc++ and Clang is inevitable. As a consequence, the advantages of being maximally compatible with libstdc++ are only present when we can be largely *forward* compatible. This seems impossible if we have to intercept ever header to reference a GCC builtin and introduce our wrapper for it.
>
>
> At a fundamental level, I am not yet comfortable with Clang, after defining __GNUC__ and acting as a heavily compatible compiler with GCC, going on to define builtin functions which share the same name but incompatible semantics with GCC builtins. This seems like a recipe for a long string of puzzling compilation failures, user frustration, and impeded adoption of Clang.
>
> I think we should continue with the long standing policy of, when reasonable and tenable, supporting the GCC builtins with their names, and allowing users to not care. Then, under a *different* naming pattern, and guarded by proper __has_feature macros etc, we should provide Clang-specific extensions which have the semantics we would rather see.
>
> While a realize this isn't ideal, and it requires renaming currently supported intrinsics I see few options left to us:
> 1) We haven't released with these intrinsics
> 2) GCC has released with its intrinsics
> 3) We failed to discuss our intrinsics sufficiently with the GCC folks to get them to implement the same set
> 4) GCC folks failed to discuss theirs with us sufficiently to get us to implement the same set
> 5) We need something for Clang v3.1, or it is instantly un-usable with GCC 4.7 and later.
>
> I don't like any of this, and it isn't how I would wish for things to proceed if I had it to do again, but this is where we are. We need to realize that, today, I cannot compile any meaningful C++98 code with the default C++ standard library on Linux (after a distro picks up GCC 4.7) and Clang. I do not think that is an acceptable state to release Clang under.

Perhaps we need to stake out another namespace for clang atomic intrinsics.  Our current prefix is:

__atomic_

Suggestions:

__clang_atomic_
__clng_atmc_
_ClangAtomic_
_CLNGatomic_
_CLNG_ATM_
_ClngAtm_

The final suggestion has the same length as our current prefix.

Howard

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: __atomic_* support for gcc 4.7 compatibility

Howard Hinnant
On Apr 10, 2012, at 7:59 PM, Howard Hinnant wrote:

> On Apr 10, 2012, at 7:24 PM, Chandler Carruth wrote:
>
>> I want to call out this point:
>>
>> On Wed, Apr 11, 2012 at 12:25 AM, Richard Smith <[hidden email]> wrote:
>> This approach, including the need to interpose in front of <atomic> and <shared_ptr> (and any other user of the atomics builtins in libstdc++) and detect whether we're about to include a header from libstdc++, seems sufficiently fragile that I'm still in favour of introducing GNU-compatible builtins.
>>
>> I think this is setting ourselves up for an endless game of catchup. What is worse, our users will suffer.
>>
>> Version skew between libstdc++ and Clang is inevitable. As a consequence, the advantages of being maximally compatible with libstdc++ are only present when we can be largely *forward* compatible. This seems impossible if we have to intercept ever header to reference a GCC builtin and introduce our wrapper for it.
>>
>>
>> At a fundamental level, I am not yet comfortable with Clang, after defining __GNUC__ and acting as a heavily compatible compiler with GCC, going on to define builtin functions which share the same name but incompatible semantics with GCC builtins. This seems like a recipe for a long string of puzzling compilation failures, user frustration, and impeded adoption of Clang.
>>
>> I think we should continue with the long standing policy of, when reasonable and tenable, supporting the GCC builtins with their names, and allowing users to not care. Then, under a *different* naming pattern, and guarded by proper __has_feature macros etc, we should provide Clang-specific extensions which have the semantics we would rather see.
>>
>> While a realize this isn't ideal, and it requires renaming currently supported intrinsics I see few options left to us:
>> 1) We haven't released with these intrinsics
>> 2) GCC has released with its intrinsics
>> 3) We failed to discuss our intrinsics sufficiently with the GCC folks to get them to implement the same set
>> 4) GCC folks failed to discuss theirs with us sufficiently to get us to implement the same set
>> 5) We need something for Clang v3.1, or it is instantly un-usable with GCC 4.7 and later.
>>
>> I don't like any of this, and it isn't how I would wish for things to proceed if I had it to do again, but this is where we are. We need to realize that, today, I cannot compile any meaningful C++98 code with the default C++ standard library on Linux (after a distro picks up GCC 4.7) and Clang. I do not think that is an acceptable state to release Clang under.
>
> Perhaps we need to stake out another namespace for clang atomic intrinsics.  Our current prefix is:
>
> __atomic_
>
> Suggestions:
>
> __clang_atomic_
> __clng_atmc_
> _ClangAtomic_
> _CLNGatomic_
> _CLNG_ATM_
> _ClngAtm_
>
> The final suggestion has the same length as our current prefix.


Or even shorter: :-)

__atomx_

Howard

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: __atomic_* support for gcc 4.7 compatibility

Chandler Carruth
On Wed, Apr 11, 2012 at 1:03 AM, Howard Hinnant <[hidden email]> wrote:
On Apr 10, 2012, at 7:59 PM, Howard Hinnant wrote:

> On Apr 10, 2012, at 7:24 PM, Chandler Carruth wrote:
>
>> I want to call out this point:
>>
>> On Wed, Apr 11, 2012 at 12:25 AM, Richard Smith <[hidden email]> wrote:
>> This approach, including the need to interpose in front of <atomic> and <shared_ptr> (and any other user of the atomics builtins in libstdc++) and detect whether we're about to include a header from libstdc++, seems sufficiently fragile that I'm still in favour of introducing GNU-compatible builtins.
>>
>> I think this is setting ourselves up for an endless game of catchup. What is worse, our users will suffer.
>>
>> Version skew between libstdc++ and Clang is inevitable. As a consequence, the advantages of being maximally compatible with libstdc++ are only present when we can be largely *forward* compatible. This seems impossible if we have to intercept ever header to reference a GCC builtin and introduce our wrapper for it.
>>
>>
>> At a fundamental level, I am not yet comfortable with Clang, after defining __GNUC__ and acting as a heavily compatible compiler with GCC, going on to define builtin functions which share the same name but incompatible semantics with GCC builtins. This seems like a recipe for a long string of puzzling compilation failures, user frustration, and impeded adoption of Clang.
>>
>> I think we should continue with the long standing policy of, when reasonable and tenable, supporting the GCC builtins with their names, and allowing users to not care. Then, under a *different* naming pattern, and guarded by proper __has_feature macros etc, we should provide Clang-specific extensions which have the semantics we would rather see.
>>
>> While a realize this isn't ideal, and it requires renaming currently supported intrinsics I see few options left to us:
>> 1) We haven't released with these intrinsics
>> 2) GCC has released with its intrinsics
>> 3) We failed to discuss our intrinsics sufficiently with the GCC folks to get them to implement the same set
>> 4) GCC folks failed to discuss theirs with us sufficiently to get us to implement the same set
>> 5) We need something for Clang v3.1, or it is instantly un-usable with GCC 4.7 and later.
>>
>> I don't like any of this, and it isn't how I would wish for things to proceed if I had it to do again, but this is where we are. We need to realize that, today, I cannot compile any meaningful C++98 code with the default C++ standard library on Linux (after a distro picks up GCC 4.7) and Clang. I do not think that is an acceptable state to release Clang under.
>
> Perhaps we need to stake out another namespace for clang atomic intrinsics.  Our current prefix is:
>
> __atomic_
>
> Suggestions:
>
> __clang_atomic_
> __clng_atmc_
> _ClangAtomic_
> _CLNGatomic_
> _CLNG_ATM_
> _ClngAtm_
>
> The final suggestion has the same length as our current prefix.


Or even shorter: :-)

__atomx_

I am quite lazy when it comes to typing, but perhaps here is not the time to save characters. ;] I would vote:

__clang_atomic_

or, based on the discussion between David and Richard, perhaps:

__c11_atomic_

to call out their close correspondance with the c11 spec.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: __atomic_* support for gcc 4.7 compatibility

Jean-Daniel Dupas-2

Le 11 avr. 2012 à 02:08, Chandler Carruth a écrit :

On Wed, Apr 11, 2012 at 1:03 AM, Howard Hinnant <[hidden email]> wrote:
On Apr 10, 2012, at 7:59 PM, Howard Hinnant wrote:

> On Apr 10, 2012, at 7:24 PM, Chandler Carruth wrote:
>
>> I want to call out this point:
>>
>> On Wed, Apr 11, 2012 at 12:25 AM, Richard Smith <[hidden email]> wrote:
>> This approach, including the need to interpose in front of <atomic> and <shared_ptr> (and any other user of the atomics builtins in libstdc++) and detect whether we're about to include a header from libstdc++, seems sufficiently fragile that I'm still in favour of introducing GNU-compatible builtins.
>>
>> I think this is setting ourselves up for an endless game of catchup. What is worse, our users will suffer.
>>
>> Version skew between libstdc++ and Clang is inevitable. As a consequence, the advantages of being maximally compatible with libstdc++ are only present when we can be largely *forward* compatible. This seems impossible if we have to intercept ever header to reference a GCC builtin and introduce our wrapper for it.
>>
>>
>> At a fundamental level, I am not yet comfortable with Clang, after defining __GNUC__ and acting as a heavily compatible compiler with GCC, going on to define builtin functions which share the same name but incompatible semantics with GCC builtins. This seems like a recipe for a long string of puzzling compilation failures, user frustration, and impeded adoption of Clang.
>>
>> I think we should continue with the long standing policy of, when reasonable and tenable, supporting the GCC builtins with their names, and allowing users to not care. Then, under a *different* naming pattern, and guarded by proper __has_feature macros etc, we should provide Clang-specific extensions which have the semantics we would rather see.
>>
>> While a realize this isn't ideal, and it requires renaming currently supported intrinsics I see few options left to us:
>> 1) We haven't released with these intrinsics
>> 2) GCC has released with its intrinsics
>> 3) We failed to discuss our intrinsics sufficiently with the GCC folks to get them to implement the same set
>> 4) GCC folks failed to discuss theirs with us sufficiently to get us to implement the same set
>> 5) We need something for Clang v3.1, or it is instantly un-usable with GCC 4.7 and later.
>>
>> I don't like any of this, and it isn't how I would wish for things to proceed if I had it to do again, but this is where we are. We need to realize that, today, I cannot compile any meaningful C++98 code with the default C++ standard library on Linux (after a distro picks up GCC 4.7) and Clang. I do not think that is an acceptable state to release Clang under.
>
> Perhaps we need to stake out another namespace for clang atomic intrinsics.  Our current prefix is:
>
> __atomic_
>
> Suggestions:
>
> __clang_atomic_
> __clng_atmc_
> _ClangAtomic_
> _CLNGatomic_
> _CLNG_ATM_
> _ClngAtm_
>
> The final suggestion has the same length as our current prefix.


Or even shorter: :-)

__atomx_

I am quite lazy when it comes to typing, but perhaps here is not the time to save characters. ;] I would vote:

__clang_atomic_


The prefix length should not be an issue.
As David said, these builtins should be reserved to implement <stdatomic.h> and <atomic>, so should never have to call them explicitly.

or, based on the discussion between David and Richard, perhaps:

__c11_atomic_

to call out their close correspondance with the c11 spec.


-- Jean-Daniel





_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: __atomic_* support for gcc 4.7 compatibility

Marc J. Driftmeyer
In reply to this post by Chandler Carruth
You folks do all the heavy lifting and thus are fully/well versed in the syntax and phonetics of the grammar you choose, but to put it bluntly if English became a chicken scratch language it never would have developed fully [though other languages would challenge that fully developed assertion on my part].

__clang_atomic_ is clearly readable and discernible, and I could care less about saving a few bytes in characters. Most editors can also make custom tags that trigger the entire string with a simple combination of key-strokes for an action to be performed.

This is one of the reasons I personally have a love/hate relationship with Computer Science and another reason Mechanical Engineering is my first and more beloved degree.

It's also another reason I was glad to work at NeXT and Apple with the knowledge that Objective-C is such a readable programming language, unlike the chicken scratch often found in C/C++.

Sincerely,

- Marc J. Driftmeyer

On 04/10/2012 05:08 PM, Chandler Carruth wrote:
On Wed, Apr 11, 2012 at 1:03 AM, Howard Hinnant <[hidden email]> wrote:
On Apr 10, 2012, at 7:59 PM, Howard Hinnant wrote:

> On Apr 10, 2012, at 7:24 PM, Chandler Carruth wrote:
>
>> I want to call out this point:
>>
>> On Wed, Apr 11, 2012 at 12:25 AM, Richard Smith <[hidden email]> wrote:
>> This approach, including the need to interpose in front of <atomic> and <shared_ptr> (and any other user of the atomics builtins in libstdc++) and detect whether we're about to include a header from libstdc++, seems sufficiently fragile that I'm still in favour of introducing GNU-compatible builtins.
>>
>> I think this is setting ourselves up for an endless game of catchup. What is worse, our users will suffer.
>>
>> Version skew between libstdc++ and Clang is inevitable. As a consequence, the advantages of being maximally compatible with libstdc++ are only present when we can be largely *forward* compatible. This seems impossible if we have to intercept ever header to reference a GCC builtin and introduce our wrapper for it.
>>
>>
>> At a fundamental level, I am not yet comfortable with Clang, after defining __GNUC__ and acting as a heavily compatible compiler with GCC, going on to define builtin functions which share the same name but incompatible semantics with GCC builtins. This seems like a recipe for a long string of puzzling compilation failures, user frustration, and impeded adoption of Clang.
>>
>> I think we should continue with the long standing policy of, when reasonable and tenable, supporting the GCC builtins with their names, and allowing users to not care. Then, under a *different* naming pattern, and guarded by proper __has_feature macros etc, we should provide Clang-specific extensions which have the semantics we would rather see.
>>
>> While a realize this isn't ideal, and it requires renaming currently supported intrinsics I see few options left to us:
>> 1) We haven't released with these intrinsics
>> 2) GCC has released with its intrinsics
>> 3) We failed to discuss our intrinsics sufficiently with the GCC folks to get them to implement the same set
>> 4) GCC folks failed to discuss theirs with us sufficiently to get us to implement the same set
>> 5) We need something for Clang v3.1, or it is instantly un-usable with GCC 4.7 and later.
>>
>> I don't like any of this, and it isn't how I would wish for things to proceed if I had it to do again, but this is where we are. We need to realize that, today, I cannot compile any meaningful C++98 code with the default C++ standard library on Linux (after a distro picks up GCC 4.7) and Clang. I do not think that is an acceptable state to release Clang under.
>
> Perhaps we need to stake out another namespace for clang atomic intrinsics.  Our current prefix is:
>
> __atomic_
>
> Suggestions:
>
> __clang_atomic_
> __clng_atmc_
> _ClangAtomic_
> _CLNGatomic_
> _CLNG_ATM_
> _ClngAtm_
>
> The final suggestion has the same length as our current prefix.


Or even shorter: :-)

__atomx_

I am quite lazy when it comes to typing, but perhaps here is not the time to save characters. ;] I would vote:

__clang_atomic_

or, based on the discussion between David and Richard, perhaps:

__c11_atomic_

to call out their close correspondance with the c11 spec.


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

--
Marc J. Driftmeyer
Email :: [hidden email]
Web :: http://www.reanimality.com
Cell :: (509) 435-5212

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

mjd.vcf (434 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: RFC: __atomic_* support for gcc 4.7 compatibility

Seth Cantrell
In reply to this post by Chandler Carruth
Calling out the correspondence with the C11 spec with __c11_atomic_ does seem desirable. Also maybe it's better for adoption by gcc as well, if they choose to add builtins that correspond to C11?

On Apr 10, 2012, at 8:08 PM, Chandler Carruth wrote:

On Wed, Apr 11, 2012 at 1:03 AM, Howard Hinnant <[hidden email]> wrote:
On Apr 10, 2012, at 7:59 PM, Howard Hinnant wrote:

> On Apr 10, 2012, at 7:24 PM, Chandler Carruth wrote:
>
>> I want to call out this point:
>>
>> On Wed, Apr 11, 2012 at 12:25 AM, Richard Smith <[hidden email]> wrote:
>> This approach, including the need to interpose in front of <atomic> and <shared_ptr> (and any other user of the atomics builtins in libstdc++) and detect whether we're about to include a header from libstdc++, seems sufficiently fragile that I'm still in favour of introducing GNU-compatible builtins.
>>
>> I think this is setting ourselves up for an endless game of catchup. What is worse, our users will suffer.
>>
>> Version skew between libstdc++ and Clang is inevitable. As a consequence, the advantages of being maximally compatible with libstdc++ are only present when we can be largely *forward* compatible. This seems impossible if we have to intercept ever header to reference a GCC builtin and introduce our wrapper for it.
>>
>>
>> At a fundamental level, I am not yet comfortable with Clang, after defining __GNUC__ and acting as a heavily compatible compiler with GCC, going on to define builtin functions which share the same name but incompatible semantics with GCC builtins. This seems like a recipe for a long string of puzzling compilation failures, user frustration, and impeded adoption of Clang.
>>
>> I think we should continue with the long standing policy of, when reasonable and tenable, supporting the GCC builtins with their names, and allowing users to not care. Then, under a *different* naming pattern, and guarded by proper __has_feature macros etc, we should provide Clang-specific extensions which have the semantics we would rather see.
>>
>> While a realize this isn't ideal, and it requires renaming currently supported intrinsics I see few options left to us:
>> 1) We haven't released with these intrinsics
>> 2) GCC has released with its intrinsics
>> 3) We failed to discuss our intrinsics sufficiently with the GCC folks to get them to implement the same set
>> 4) GCC folks failed to discuss theirs with us sufficiently to get us to implement the same set
>> 5) We need something for Clang v3.1, or it is instantly un-usable with GCC 4.7 and later.
>>
>> I don't like any of this, and it isn't how I would wish for things to proceed if I had it to do again, but this is where we are. We need to realize that, today, I cannot compile any meaningful C++98 code with the default C++ standard library on Linux (after a distro picks up GCC 4.7) and Clang. I do not think that is an acceptable state to release Clang under.
>
> Perhaps we need to stake out another namespace for clang atomic intrinsics.  Our current prefix is:
>
> __atomic_
>
> Suggestions:
>
> __clang_atomic_
> __clng_atmc_
> _ClangAtomic_
> _CLNGatomic_
> _CLNG_ATM_
> _ClngAtm_
>
> The final suggestion has the same length as our current prefix.


Or even shorter: :-)

__atomx_

I am quite lazy when it comes to typing, but perhaps here is not the time to save characters. ;] I would vote:

__clang_atomic_

or, based on the discussion between David and Richard, perhaps:

__c11_atomic_

to call out their close correspondance with the c11 spec.
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: __atomic_* support for gcc 4.7 compatibility

Hal Finkel
On Tue, 10 Apr 2012 20:54:30 -0400
Seth Cantrell <[hidden email]> wrote:

> Calling out the correspondence with the C11 spec with __c11_atomic_
> does seem desirable.

I agree. Another alternative is to just use __builtin_.

 -Hal

 Also maybe it's better for adoption by gcc as

> well, if they choose to add builtins that correspond to C11?
>
> On Apr 10, 2012, at 8:08 PM, Chandler Carruth wrote:
>
> > On Wed, Apr 11, 2012 at 1:03 AM, Howard Hinnant
> > <[hidden email]> wrote: On Apr 10, 2012, at 7:59 PM, Howard
> > Hinnant wrote:
> >
> > > On Apr 10, 2012, at 7:24 PM, Chandler Carruth wrote:
> > >
> > >> I want to call out this point:
> > >>
> > >> On Wed, Apr 11, 2012 at 12:25 AM, Richard Smith
> > >> <[hidden email]> wrote: This approach, including the need
> > >> to interpose in front of <atomic> and <shared_ptr> (and any
> > >> other user of the atomics builtins in libstdc++) and detect
> > >> whether we're about to include a header from libstdc++, seems
> > >> sufficiently fragile that I'm still in favour of introducing
> > >> GNU-compatible builtins.
> > >>
> > >> I think this is setting ourselves up for an endless game of
> > >> catchup. What is worse, our users will suffer.
> > >>
> > >> Version skew between libstdc++ and Clang is inevitable. As a
> > >> consequence, the advantages of being maximally compatible with
> > >> libstdc++ are only present when we can be largely *forward*
> > >> compatible. This seems impossible if we have to intercept ever
> > >> header to reference a GCC builtin and introduce our wrapper for
> > >> it.
> > >>
> > >>
> > >> At a fundamental level, I am not yet comfortable with Clang,
> > >> after defining __GNUC__ and acting as a heavily compatible
> > >> compiler with GCC, going on to define builtin functions which
> > >> share the same name but incompatible semantics with GCC
> > >> builtins. This seems like a recipe for a long string of puzzling
> > >> compilation failures, user frustration, and impeded adoption of
> > >> Clang.
> > >>
> > >> I think we should continue with the long standing policy of,
> > >> when reasonable and tenable, supporting the GCC builtins with
> > >> their names, and allowing users to not care. Then, under a
> > >> *different* naming pattern, and guarded by proper __has_feature
> > >> macros etc, we should provide Clang-specific extensions which
> > >> have the semantics we would rather see.
> > >>
> > >> While a realize this isn't ideal, and it requires renaming
> > >> currently supported intrinsics I see few options left to us: 1)
> > >> We haven't released with these intrinsics 2) GCC has released
> > >> with its intrinsics 3) We failed to discuss our intrinsics
> > >> sufficiently with the GCC folks to get them to implement the
> > >> same set 4) GCC folks failed to discuss theirs with us
> > >> sufficiently to get us to implement the same set 5) We need
> > >> something for Clang v3.1, or it is instantly un-usable with GCC
> > >> 4.7 and later.
> > >>
> > >> I don't like any of this, and it isn't how I would wish for
> > >> things to proceed if I had it to do again, but this is where we
> > >> are. We need to realize that, today, I cannot compile any
> > >> meaningful C++98 code with the default C++ standard library on
> > >> Linux (after a distro picks up GCC 4.7) and Clang. I do not
> > >> think that is an acceptable state to release Clang under.
> > >
> > > Perhaps we need to stake out another namespace for clang atomic
> > > intrinsics.  Our current prefix is:
> > >
> > > __atomic_
> > >
> > > Suggestions:
> > >
> > > __clang_atomic_
> > > __clng_atmc_
> > > _ClangAtomic_
> > > _CLNGatomic_
> > > _CLNG_ATM_
> > > _ClngAtm_
> > >
> > > The final suggestion has the same length as our current prefix.
> >
> >
> > Or even shorter: :-)
> >
> > __atomx_
> >
> > I am quite lazy when it comes to typing, but perhaps here is not
> > the time to save characters. ;] I would vote:
> >
> > __clang_atomic_
> >
> > or, based on the discussion between David and Richard, perhaps:
> >
> > __c11_atomic_
> >
> > to call out their close correspondance with the c11 spec.
> > _______________________________________________
> > cfe-dev mailing list
> > [hidden email]
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>



--
Hal Finkel
Postdoctoral Appointee
Leadership Computing Facility
Argonne National Laboratory
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: __atomic_* support for gcc 4.7 compatibility

jyasskin
In reply to this post by Howard Hinnant
On Tue, Apr 10, 2012 at 3:20 PM, Howard Hinnant <[hidden email]> wrote:

> On Apr 10, 2012, at 6:11 PM, David Chisnall wrote:
>
>> On 10 Apr 2012, at 22:57, Howard Hinnant wrote:
>>
>>> David, could you revert the initialization statements to use either plain C++, or the __atomic_init intrinsic?
>>
>> Sorry, I thought I'd done that already when I fixed initialisation of _Atomic types in clang.  I'm just about to fall asleep, but I'll do it in the morning.
>
> Of course.  I'd much rather you be awake when you do this. :-)
>
>>
>> __atomic_init() is still required for atomic_init() (in both C11 and C++11) - these are currently using __atomic_store, in libc++, so I'll fix them at the same time.
>
> template <class _Tp>
> inline _LIBCPP_INLINE_VISIBILITY
> void
> atomic_init(atomic<_Tp>* __o, _Tp __d)
> {
>    __o->__a_ = __d;
> }
>
> Not right?

If __o->__a_ is an _Atomic(_Tp), then C11 says "Loads and stores of
objects with atomic types are done with memory_order_seq_cst
semantics." (6.2.6.1), and the simple assignment gives you the
equivalent of atomic_store(&__o->__a_, __d, memory_order_seq_cst).

You could get the effect of atomic_init with a memcpy, but only by
knowing the _Atomic(_Tp)'s layout on the target platform.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: RFC: __atomic_* support for gcc 4.7 compatibility

David Chisnall-2
In reply to this post by Howard Hinnant
On 10 Apr 2012, at 23:20, Howard Hinnant wrote:

> Of course.  I'd much rather you be awake when you do this. :-)

I thought you might...

>> __atomic_init() is still required for atomic_init() (in both C11 and C++11) - these are currently using __atomic_store, in libc++, so I'll fix them at the same time.
>
> template <class _Tp>
> inline _LIBCPP_INLINE_VISIBILITY
> void
> atomic_init(atomic<_Tp>* __o, _Tp __d)
> {
>    __o->__a_ = __d;
> }
>
> Not right?

No, because assignments to _Atomic() variables with = are atomic, with sequentially consistent semantics.  This is more or less the opposite of what we want.

David
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
12