libc++: help diagnosing the following std::atomic compile error

Hello all,

I am trying to help along the recently added Boost.Lockfree (1) library utilize libc++'s std::atomic, as it is currently using Boost.Atomic, and an emulated, locking implementation at that. is currently used for this library for modern gcc and msvc compilers, but not yet clang / libc++.

(note: to enable for boost::lockfree, first apply this patch: https://svn.boost.org/trac/boost/attachment/ticket/8593/lockfree_atomic_libcpp.patch )

The problem can be seen in the compile errors for this simple program:

#include “boost/lockfree/queue.hpp”

int main(int argc, const char * argv[])
{

boost::lockfree::queue q( 1 );

return 0;
}

The diagnostic is:

In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/memory:606:
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/atomic:623:58: error: no viable conversion from ‘boost::lockfree::detail::tagged_ptr<boost::lockfree::queue<int, boost::parameter::void_, boost::parameter::void_, boost::parameter::void_>::node>’ to ‘Atomic(boost::lockfree::detail::tagged_ptr<boost::lockfree::queue<int, boost::parameter::void, boost::parameter::void_, boost::parameter::void_>::node>)’
_LIBCPP_CONSTEXPR __atomic_base(_Tp __d) _NOEXCEPT : _a(__d) {}
^ ~~~
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/atomic:727:51: note: in instantiation of member function ‘std::_1::atomic_base<boost::lockfree::detail::tagged_ptr<boost::lockfree::queue<int, boost::parameter::void, boost::parameter::void, boost::parameter::void>::node>, false>::__atomic_base’ requested here
_LIBCPP_CONSTEXPR atomic(_Tp __d) NOEXCEPT : base(d) {}
^
…/…/…/…/…/cinder-dev/boost/boost/lockfree/queue.hpp:192:9: note: in instantiation of member function 'std::1::atomic<boost::lockfree::detail::tagged_ptr<boost::lockfree::queue<int, boost::parameter::void, boost::parameter::void, boost::parameter::void
>::node> >::atomic’ requested here
head
(tagged_node_handle(0, 0)),
^
/Users/richeakin/code/cinder/audio-rewrite/audio2/test/BasicTest/xcode/…/src/BasicTestApp.cpp:75:30: note: in instantiation of member function 'boost::lockfree::queue<int, boost::parameter::void
, boost::parameter::void
, boost::parameter::void
>::queue’ requested here
boost::lockfree::queue q( 1 );
^
…/…/…/…/…/cinder-dev/boost/boost/lockfree/detail/tagged_ptr_dcas.hpp:112:5: note: candidate function
operator bool(void) const
^

My understanding is (please correct me if wrong) that _Atomic(_Tp) is a directive to the compiler, which is replaced with atomic the true atomic instructions for each given architecture. As such, it doesn’t know how to replace boost::lockfree::detail::tagged_ptr<…> with size_t, or whatever other atomic value lockfree expects. But, this is where my understanding of this sort of template metaprogramming reaches its end, I just can’t tell why this trips up with libc++ but not with vc11.

Can anyone see where the translation falls short, or have suggestions on how to proceeed?

Thank you kindly,
Rich

1: http://www.boost.org/doc/libs/1_53_0/doc/html/lockfree.html

This looks like a clang bug to me. _Atomic seems to not be set up to deal with non-integral types.

Here is a simplified version of Rich's test:

#include <atomic>

struct A
{
    A(int) {}
};

int
main()
{
    std::atomic<A> q(A(1));
}

Howard

Hello all,

I am trying to help along the recently added Boost.Lockfree (1) library utilize libc++'s std::atomic, as it is currently using Boost.Atomic, and an emulated, locking implementation at that. is currently used for this library for modern gcc and msvc compilers, but not yet clang / libc++.

(note: to enable for boost::lockfree, first apply this patch: https://svn.boost.org/trac/boost/attachment/ticket/8593/lockfree_atomic_libcpp.patch )

The problem can be seen in the compile errors for this simple program:

#include “boost/lockfree/queue.hpp”

int main(int argc, const char * argv[])
{
boost::lockfree::queue q( 1 );
return 0;
}

The diagnostic is:

In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/memory:606:
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/atomic:623:58: error: no viable conversion from ‘boost::lockfree::detail::tagged_ptr<boost::lockfree::queue<int, boost::parameter::void_, boost::parameter::void_, boost::parameter::void_>::node>’ to ‘Atomic(boost::lockfree::detail::tagged_ptr<boost::lockfree::queue<int, boost::parameter::void, boost::parameter::void_, boost::parameter::void_>::node>)’
_LIBCPP_CONSTEXPR __atomic_base(_Tp __d) _NOEXCEPT : _a(__d) {}
^ ~~~
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/atomic:727:51: note: in instantiation of member function ‘std::_1::atomic_base<boost::lockfree::detail::tagged_ptr<boost::lockfree::queue<int, boost::parameter::void, boost::parameter::void, boost::parameter::void>::node>, false>::__atomic_base’ requested here
_LIBCPP_CONSTEXPR atomic(_Tp __d) NOEXCEPT : base(d) {}
^
…/…/…/…/…/cinder-dev/boost/boost/lockfree/queue.hpp:192:9: note: in instantiation of member function 'std::1::atomic<boost::lockfree::detail::tagged_ptr<boost::lockfree::queue<int, boost::parameter::void, boost::parameter::void, boost::parameter::void
>::node> >::atomic’ requested here
head
(tagged_node_handle(0, 0)),
^
/Users/richeakin/code/cinder/audio-rewrite/audio2/test/BasicTest/xcode/…/src/BasicTestApp.cpp:75:30: note: in instantiation of member function 'boost::lockfree::queue<int, boost::parameter::void
, boost::parameter::void
, boost::parameter::void
>::queue’ requested here
boost::lockfree::queue q( 1 );
^
…/…/…/…/…/cinder-dev/boost/boost/lockfree/detail/tagged_ptr_dcas.hpp:112:5: note: candidate function
operator bool(void) const
^

My understanding is (please correct me if wrong) that _Atomic(_Tp) is a directive to the compiler, which is replaced with atomic the true atomic instructions for each given architecture. As such, it doesn’t know how to replace boost::lockfree::detail::tagged_ptr<…> with size_t, or whatever other atomic value lockfree expects. But, this is where my understanding of this sort of template metaprogramming reaches its end, I just can’t tell why this trips up with libc++ but not with vc11.

Can anyone see where the translation falls short, or have suggestions on how to proceeed?

This looks like a clang bug to me. _Atomic seems to not be set up to deal with non-integral types.

I don’t think this is clearly a clang bug. We support _Atomic(T) in C++ mode as an extension, but that extension currently does not extend to allowing an _Atomic(T) to be initialized as if it were a T, if T is of class type. We can only reasonably support this when T’s corresponding constructor is trivial, but I believe that’s all you actually need here, right? (We don’t want to allow calling member functions on an object of type T which is wrapped within an _Atomic(…)).

C++11 requires only that T be trivially copyable. So it seems like this should work:

#include <atomic>
#include <type_traits>

struct A
{
    int i_;

    A(int i) : i_(i) {}
};

static_assert(std::is_trivially_copyable<A>::value, "");

int
main()
{
    std::atomic<A> q(A(1));
}

The _Atomic type specifier was added to libc++ by this commit:

I think there are basically two long-term options and one short-term workaround:

  1. Use T rather than _Atomic(T), manually ensure that std::atomic has the right alignment, and use the _atomic* builtins instead of the _c11_atomic* builtins (this is the approach which libstdc++ takes), or
  2. Get someone to implement initialization support for _Atomic(T), where T is a class type where the selected copy/move constructor is trivial. I don’t have time to work on this right now, I’m afraid.

Workaround: Use __c11_atomic_init instead of initializing the member in the mem-initializer-list, for now at least. This has the disadvantage that you can’t mark the constructor as ‘constexpr’, but you could restrict this to the case of non-scalar T to avoid regressing.

Thanks. Do we have any documentation for the __atomic_* builtins?

In file included from test.cpp:1:
/Users/hhinnant/Development/temp_libcxx/include/atomic:564:50: error: too few arguments to function call, expected 2, have 1
        {return __atomic_is_lock_free(sizeof(_Tp));}
                ~~~~~~~~~~~~~~~~~~~~~ ^
/Users/hhinnant/Development/temp_libcxx/include/atomic:567:50: error: too few arguments to function call, expected 2, have 1
        {return __atomic_is_lock_free(sizeof(_Tp));}
                ~~~~~~~~~~~~~~~~~~~~~ ^
/Users/hhinnant/Development/temp_libcxx/include/atomic:1362:51: error: too few arguments to function call, expected 4, have 3
        {return __atomic_exchange(&__a_, true, __m);}
                ~~~~~~~~~~~~~~~~~ ^
/Users/hhinnant/Development/temp_libcxx/include/atomic:1365:51: error: too few arguments to function call, expected 4, have 3
        {return __atomic_exchange(&__a_, true, __m);}
                ~~~~~~~~~~~~~~~~~ ^
4 errors generated.

Howard

Hello all,

I am trying to help along the recently added Boost.Lockfree (1) library utilize libc++'s std::atomic, as it is currently using Boost.Atomic, and an emulated, locking implementation at that. is currently used for this library for modern gcc and msvc compilers, but not yet clang / libc++.

(note: to enable for boost::lockfree, first apply this patch: https://svn.boost.org/trac/boost/attachment/ticket/8593/lockfree_atomic_libcpp.patch )

The problem can be seen in the compile errors for this simple program:

#include “boost/lockfree/queue.hpp”

int main(int argc, const char * argv[])
{
boost::lockfree::queue q( 1 );
return 0;
}

The diagnostic is:

In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/memory:606:
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/atomic:623:58: error: no viable conversion from ‘boost::lockfree::detail::tagged_ptr<boost::lockfree::queue<int, boost::parameter::void_, boost::parameter::void_, boost::parameter::void_>::node>’ to ‘Atomic(boost::lockfree::detail::tagged_ptr<boost::lockfree::queue<int, boost::parameter::void, boost::parameter::void_, boost::parameter::void_>::node>)’
_LIBCPP_CONSTEXPR __atomic_base(_Tp __d) _NOEXCEPT : _a(__d) {}
^ ~~~
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/atomic:727:51: note: in instantiation of member function ‘std::_1::atomic_base<boost::lockfree::detail::tagged_ptr<boost::lockfree::queue<int, boost::parameter::void, boost::parameter::void, boost::parameter::void>::node>, false>::__atomic_base’ requested here
_LIBCPP_CONSTEXPR atomic(_Tp __d) NOEXCEPT : base(d) {}
^
…/…/…/…/…/cinder-dev/boost/boost/lockfree/queue.hpp:192:9: note: in instantiation of member function 'std::1::atomic<boost::lockfree::detail::tagged_ptr<boost::lockfree::queue<int, boost::parameter::void, boost::parameter::void, boost::parameter::void
>::node> >::atomic’ requested here
head
(tagged_node_handle(0, 0)),
^
/Users/richeakin/code/cinder/audio-rewrite/audio2/test/BasicTest/xcode/…/src/BasicTestApp.cpp:75:30: note: in instantiation of member function 'boost::lockfree::queue<int, boost::parameter::void
, boost::parameter::void
, boost::parameter::void
>::queue’ requested here
boost::lockfree::queue q( 1 );
^
…/…/…/…/…/cinder-dev/boost/boost/lockfree/detail/tagged_ptr_dcas.hpp:112:5: note: candidate function
operator bool(void) const
^

My understanding is (please correct me if wrong) that _Atomic(_Tp) is a directive to the compiler, which is replaced with atomic the true atomic instructions for each given architecture. As such, it doesn’t know how to replace boost::lockfree::detail::tagged_ptr<…> with size_t, or whatever other atomic value lockfree expects. But, this is where my understanding of this sort of template metaprogramming reaches its end, I just can’t tell why this trips up with libc++ but not with vc11.

Can anyone see where the translation falls short, or have suggestions on how to proceeed?

This looks like a clang bug to me. _Atomic seems to not be set up to deal with non-integral types.

I don’t think this is clearly a clang bug. We support _Atomic(T) in C++ mode as an extension, but that extension currently does not extend to allowing an _Atomic(T) to be initialized as if it were a T, if T is of class type. We can only reasonably support this when T’s corresponding constructor is trivial, but I believe that’s all you actually need here, right? (We don’t want to allow calling member functions on an object of type T which is wrapped within an _Atomic(…)).

C++11 requires only that T be trivially copyable. So it seems like this should work:

#include
#include <type_traits>

struct A
{
int i_;

A(int i) : i_(i) {}
};

static_assert(std::is_trivially_copyable::value, “”);

int
main()
{
std::atomic
q(A(1));
}

The _Atomic type specifier was added to libc++ by this commit:


r146865 | theraven | 2011-12-19 06:44:20 -0500 (Mon, 19 Dec 2011) | 7 lines

Some fixes to operations to explicitly use atomic types and operations.

The integral types now work with clang trunk (if you remove the guard), although we’re still missing an intrinsic for initialising atomics (needed for C1x too).

Howard: Please review.

David Chisnall and I had a private conversation about this addition:

I’m not fully understanding why we need _Atomic(T). If a mutex is needed for the type, I’m thinking that would go into compiler_rt. And I’m thinking that the only characteristic that would drive the need to do this is sizeof(T).

Does clang have any documentation on _Atomic?

It’s part of the C1x spec and is supported by clang in C++ mode as an extension.

Specifically, _Atomic(T) is not required by the spec to have the same underlying representation as T (so operations on it can be guarded by an associated mutex or something lighter - e.g. some architectures only permit atomic operations on small values - e.g. only on 1 bit - and so larger values can be guarded by a flag in this bit rather than a full mutex). That’s also the rationale behind the atomic_flag stuff in both C and C++ - any architecture that supports the atomics stuff should support 1-bit atomic operations and these can be used to implement everything else by implementing a lightweight mutex.

I still don’t fully understand why we need _Atomic in the C++11 . However whether we do or not, I don’t really care. All I care about is that works. can’t be implemented in straight C++. It requires compiler support. I need a clang expert to advise me on what the libc++ should look like to just work.

In Oct. 2010 I wrote this as a proposal for how it should work and had an written to this spec:

http://libcxx.llvm.org/atomic_design_a.html

In late 2011 David Chisnall did the clang work to get to work and updated accordingly. In Apr 2012 you switched from _atomic* builtins to _c11_atomic* builtins.

The libc++ appears to work only for scalar types, and the C++11 spec says it should work for all trivially copyable types. I know of no changes I can make to libc++ at this time to fix that. Clang experts please advise.

I think there are basically two long-term options and one short-term workaround:

  1. Use T rather than _Atomic(T), manually ensure that std::atomic has the right alignment, and use the _atomic* builtins instead of the _c11_atomic* builtins (this is the approach which libstdc++ takes), or
  2. Get someone to implement initialization support for _Atomic(T), where T is a class type where the selected copy/move constructor is trivial. I don’t have time to work on this right now, I’m afraid.

Workaround: Use __c11_atomic_init instead of initializing the member in the mem-initializer-list, for now at least. This has the disadvantage that you can’t mark the constructor as ‘constexpr’, but you could restrict this to the case of non-scalar T to avoid regressing.

Thanks. Do we have any documentation for the _atomic* builtins?

They’re documented in the GCC manual:

http://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html

Hello all,

I am trying to help along the recently added Boost.Lockfree (1) library utilize libc++'s std::atomic, as it is currently using Boost.Atomic, and an emulated, locking implementation at that. is currently used for this library for modern gcc and msvc compilers, but not yet clang / libc++.

(note: to enable for boost::lockfree, first apply this patch: https://svn.boost.org/trac/boost/attachment/ticket/8593/lockfree_atomic_libcpp.patch )

The problem can be seen in the compile errors for this simple program:

#include “boost/lockfree/queue.hpp”

int main(int argc, const char * argv[])
{
boost::lockfree::queue q( 1 );
return 0;
}

The diagnostic is:

In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/memory:606:
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/atomic:623:58: error: no viable conversion from ‘boost::lockfree::detail::tagged_ptr<boost::lockfree::queue<int, boost::parameter::void_, boost::parameter::void_, boost::parameter::void_>::node>’ to ‘Atomic(boost::lockfree::detail::tagged_ptr<boost::lockfree::queue<int, boost::parameter::void, boost::parameter::void_, boost::parameter::void_>::node>)’
_LIBCPP_CONSTEXPR __atomic_base(_Tp __d) _NOEXCEPT : _a(__d) {}
^ ~~~
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/…/lib/c++/v1/atomic:727:51: note: in instantiation of member function ‘std::_1::atomic_base<boost::lockfree::detail::tagged_ptr<boost::lockfree::queue<int, boost::parameter::void, boost::parameter::void, boost::parameter::void>::node>, false>::__atomic_base’ requested here
_LIBCPP_CONSTEXPR atomic(_Tp __d) NOEXCEPT : base(d) {}
^
…/…/…/…/…/cinder-dev/boost/boost/lockfree/queue.hpp:192:9: note: in instantiation of member function 'std::1::atomic<boost::lockfree::detail::tagged_ptr<boost::lockfree::queue<int, boost::parameter::void, boost::parameter::void, boost::parameter::void
>::node> >::atomic’ requested here
head
(tagged_node_handle(0, 0)),
^
/Users/richeakin/code/cinder/audio-rewrite/audio2/test/BasicTest/xcode/…/src/BasicTestApp.cpp:75:30: note: in instantiation of member function 'boost::lockfree::queue<int, boost::parameter::void
, boost::parameter::void
, boost::parameter::void
>::queue’ requested here
boost::lockfree::queue q( 1 );
^
…/…/…/…/…/cinder-dev/boost/boost/lockfree/detail/tagged_ptr_dcas.hpp:112:5: note: candidate function
operator bool(void) const
^

My understanding is (please correct me if wrong) that _Atomic(_Tp) is a directive to the compiler, which is replaced with atomic the true atomic instructions for each given architecture. As such, it doesn’t know how to replace boost::lockfree::detail::tagged_ptr<…> with size_t, or whatever other atomic value lockfree expects. But, this is where my understanding of this sort of template metaprogramming reaches its end, I just can’t tell why this trips up with libc++ but not with vc11.

Can anyone see where the translation falls short, or have suggestions on how to proceeed?

This looks like a clang bug to me. _Atomic seems to not be set up to deal with non-integral types.

I don’t think this is clearly a clang bug. We support _Atomic(T) in C++ mode as an extension, but that extension currently does not extend to allowing an _Atomic(T) to be initialized as if it were a T, if T is of class type. We can only reasonably support this when T’s corresponding constructor is trivial, but I believe that’s all you actually need here, right? (We don’t want to allow calling member functions on an object of type T which is wrapped within an _Atomic(…)).

C++11 requires only that T be trivially copyable. So it seems like this should work:

#include
#include <type_traits>

struct A
{
int i_;

A(int i) : i_(i) {}
};

static_assert(std::is_trivially_copyable::value, “”);

int
main()
{
std::atomic
q(A(1));
}

The _Atomic type specifier was added to libc++ by this commit:


r146865 | theraven | 2011-12-19 06:44:20 -0500 (Mon, 19 Dec 2011) | 7 lines

Some fixes to operations to explicitly use atomic types and operations.

The integral types now work with clang trunk (if you remove the guard), although we’re still missing an intrinsic for initialising atomics (needed for C1x too).

Howard: Please review.

David Chisnall and I had a private conversation about this addition:

I’m not fully understanding why we need _Atomic(T). If a mutex is needed for the type, I’m thinking that would go into compiler_rt. And I’m thinking that the only characteristic that would drive the need to do this is sizeof(T).

Does clang have any documentation on _Atomic?

It’s part of the C1x spec and is supported by clang in C++ mode as an extension.

Specifically, _Atomic(T) is not required by the spec to have the same underlying representation as T (so operations on it can be guarded by an associated mutex or something lighter - e.g. some architectures only permit atomic operations on small values - e.g. only on 1 bit - and so larger values can be guarded by a flag in this bit rather than a full mutex). That’s also the rationale behind the atomic_flag stuff in both C and C++ - any architecture that supports the atomics stuff should support 1-bit atomic operations and these can be used to implement everything else by implementing a lightweight mutex.

I still don’t fully understand why we need _Atomic in the C++11 . However whether we do or not, I don’t really care. All I care about is that works. can’t be implemented in straight C++. It requires compiler support. I need a clang expert to advise me on what the libc++ should look like to just work.

In Oct. 2010 I wrote this as a proposal for how it should work and had an written to this spec:

http://libcxx.llvm.org/atomic_design_a.html

In late 2011 David Chisnall did the clang work to get to work and updated accordingly. In Apr 2012 you switched from _atomic* builtins to _c11_atomic* builtins.

The libc++ appears to work only for scalar types, and the C++11 spec says it should work for all trivially copyable types. I know of no changes I can make to libc++ at this time to fix that. Clang experts please advise.

I think there are basically two long-term options and one short-term workaround:

  1. Use T rather than _Atomic(T), manually ensure that std::atomic has the right alignment, and use the _atomic* builtins instead of the _c11_atomic* builtins (this is the approach which libstdc++ takes), or
  2. Get someone to implement initialization support for _Atomic(T), where T is a class type where the selected copy/move constructor is trivial. I don’t have time to work on this right now, I’m afraid.

Workaround: Use __c11_atomic_init instead of initializing the member in the mem-initializer-list, for now at least. This has the disadvantage that you can’t mark the constructor as ‘constexpr’, but you could restrict this to the case of non-scalar T to avoid regressing.

Thanks. Do we have any documentation for the _atomic* builtins?

They’re documented in the GCC manual:

http://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html

One thing to be cautious about:

struct T { char c[3]; };
static_assert(sizeof(std::atomic) == 3 && alignof(std::atomic) == 1, “under libstdc++”);
static_assert(sizeof(std::atomic) == 4 && alignof(std::atomic) == 4, “under libc++”);

Here's what I'm currently testing:

template <class _Tp, bool = is_integral<_Tp>::value && !is_same<_Tp, bool>::value>
struct __atomic_base // false
{
    _ALIGNAS_TYPE(_Atomic(_Tp)) _Tp __a_;

where _ALIGNAS_TYPE(x) will become alignas(x).

Does that look good to you?

Howard

Yup, LGTM.

Switching to the __atomic builtins could also fix
http://llvm.org/bugs/show_bug.cgi?id=16056

The GCC builtins are currently documented here
http://gcc.gnu.org/onlinedocs/gcc-4.8.0/gcc/_005f_005fatomic-Builtins.html

- Stephan

Indeed it would! Though we should teach Clang to treat _Atomic(T) as a literal type regardless.

Clang or libc++ bug for __atomic_fetch_add using pointer types?

#include <atomic>
#include <iostream>

template <class T>
T
simulated_atomic_fetch_add(T* ptr, ptrdiff_t val, int)
{
    T tmp = *ptr;
    *ptr += val;
    return tmp;
}

int
main()
{
    int* p = nullptr;
    __atomic_fetch_add(&p, 1, __ATOMIC_RELAXED);
    std::cout << (void*)p << '\n';
    p = nullptr;
    simulated_atomic_fetch_add(&p, 1, __ATOMIC_RELAXED);
    std::cout << (void*)p << '\n';
}

0x1
0x4

I.e. __atomic_fetch_add (and __atomic_fetch_sub) assumes the pointer is a pointer to char.

I can fix this in libc++, or should it be fixed in clang? The "official" spec at:

http://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html

is sloppy, and doesn't clarify. I'm happy to do whatever gcc is doing, and I have no idea what that is.

Howard

This is a weird behavior of gcc’s built-in, which clang faithfully emulates.

Ok, thanks Richard. I'll take care of it in libc++.

Howard

Please review:

I've enclosed a proposed patch for fixing <atomic>. This patch changes us from using _Atomic(T) for the stored type, to just T, but aligned to _Atomic(T). It also switches from the __c11_atomic intrinsics to the gcc __atomic intrinsics. This change exposed several silly bugs in the test suite. But most importantly it is meant to allow std::atomic to be instantiated with trivially copyable class types.

The enclosed patch passes all (modified) tests on Apple's Mountain Lion except one:

passed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics
passed 2 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.fences
passed 11 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.flag
passed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.general
passed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.lockfree
passed 2 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.order
passed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.syn
passed 5 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.generic
passed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations
passed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.arith
passed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.general
passed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.pointer
Undefined symbols for architecture x86_64:
  "___atomic_is_lock_free", referenced from:
      void test<A>() in atomic_is_lock_free-cA4Y5d.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
/Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.req/atomic_is_lock_free.pass.cpp failed to compile
Compile line was: xcrun clang++ -stdlib=libc++ -std=c++11 -I/Users/hhinnant/Development/temp_libcxx/test/support -I/Users/hhinnant/Development/temp_libcxx/include -L/Users/hhinnant/Development/temp_libcxx/lib -D_LIBCPP_STD_VER=13 -I/Users/hhinnant/Development/temp_libcxx/test/support -I/Users/hhinnant/Development/temp_libcxx/include -L/Users/hhinnant/Development/temp_libcxx/lib atomic_is_lock_free.pass.cpp
failed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.req
passed 22 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.req
passed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.templ

atomic.patch (17.6 KB)

Thank you to all for addressing this, I must say that you are very much considered awesome in the eyes of those who use the tools you are creating.

While I can’t justly review the code in this patch, I would like to test if the issue I ran into is indeed solved with it, however I ran into difficulties when applyinng the patch to my local git clone (http://llvm.org/git/libcxx.git). Nonetheless, it’s pretty clear the problem is understood and thank you again for addressing it. I will continue to test and report any issues I come across.

cheers,
Rich

The patch looks basically right. However, since you get the alignment of the inner object correct, you should pass 0 as the second parameter to __atomic_is_lock_free (that should remove your one remaining link error). Also, your placement-new expressions aren’t robust against an overloaded operator& on _Tp.

This looks like an ABI issue, but I don't imagine that we'll encounter any ABIs where the libstdc++ answer should be correct for alignof. It's debatable whether the sizeof answer is correct, as the last byte should be padding, but as the intention of C11 _Atomic() and C++11 std::atomic<> is to be compiled down (for <word size types) to single atomic load / store operations (or short load-linked, store-conditional sequences) without exposing racy issues libstdc++ answer looks wrong (I know of no architectures which can do 3-byte atomic ops, and with alignof(1) this could end up spanning two cache lines and be impossible to operate on atomically without a mutex in anything except Haswell or BlueGene/Q).

This is my concern with the GCC atomic builtins, and why I am not happy with clang supporting them or encouraging their use. There is explicitly no guarantee in C11 that T and _Atomic(T) have the same ABI and it was expected that they would not, because efficient atomic operations have stricter constraints (much stricter on some architectures, a bit stricter on x86). It is almost impossible to use the GCC builtins correctly in a situation where the ABIs differ for the atomic and non-atomic types.

David

This may still end up with the std::atomic<> types lacking padding in structures that makes it possible to act on them correctly. For example, what do you get when you do:

struct {
  std::atomic<char> x;
  char y;
};

y should be sufficiently after x that they can be operated on independently (at least from an ISA perspective - if they're in the same cache line then most RISCy CPUs will do a bit of redundant work anyway). Even though the alignment of y is 1, we (typically) should be adding 3 bytes of padding after x.

David