On Mon, May 20, 2013 at 2:07 PM, Howard Hinnant <span dir="ltr"><<a href="mailto:hhinnant@apple.com" target="_blank">hhinnant@apple.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
On May 20, 2013, at 4:44 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<br>
<br>
> On Mon, May 20, 2013 at 1:02 PM, Howard Hinnant <<a href="mailto:hhinnant@apple.com">hhinnant@apple.com</a>> wrote:<br>
> On May 20, 2013, at 3:29 PM, Rich E <<a href="mailto:reakinator@gmail.com">reakinator@gmail.com</a>> wrote:<br>
><br>
> > Hello all,<br>
> ><br>
> > 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. <atomic> is currently used for this library for modern gcc and msvc compilers, but not yet clang / libc++.<br>
> ><br>
> ><br>
> > (note: to enable <atomic> for boost::lockfree, first apply this patch: <a href="https://svn.boost.org/trac/boost/attachment/ticket/8593/lockfree_atomic_libcpp.patch" target="_blank">https://svn.boost.org/trac/boost/attachment/ticket/8593/lockfree_atomic_libcpp.patch</a> )<br>
> ><br>
> > The problem can be seen in the compile errors for this simple program:<br>
> ><br>
> > #include "boost/lockfree/queue.hpp"<br>
> ><br>
> > int main(int argc, const char * argv[])<br>
> > {<br>
> > boost::lockfree::queue<int> q( 1 );<br>
> > return 0;<br>
> > }<br>
> ><br>
> > The diagnostic is:<br>
> ><br>
> > In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/memory:606:<br>
> > /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>)'<br>
> > _LIBCPP_CONSTEXPR __atomic_base(_Tp __d) _NOEXCEPT : __a_(__d) {}<br>
> > ^ ~~~<br>
> > /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<br>
> > _LIBCPP_CONSTEXPR atomic(_Tp __d) _NOEXCEPT : __base(__d) {}<br>
> > ^<br>
> > ../../../../../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<br>
> > head_(tagged_node_handle(0, 0)),<br>
> > ^<br>
> > /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<br>
> > boost::lockfree::queue<int> q( 1 );<br>
> > ^<br>
> > ../../../../../cinder-dev/boost/boost/lockfree/detail/tagged_ptr_dcas.hpp:112:5: note: candidate function<br>
> > operator bool(void) const<br>
> > ^<br>
> ><br>
> ><br>
> > 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.<br>
> ><br>
> > Can anyone see where the translation falls short, or have suggestions on how to proceeed?<br>
><br>
> > This looks like a clang bug to me. _Atomic seems to not be set up to deal with non-integral types.<br>
><br>
> 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(...)).<br>
<br>
</div></div>C++11 requires only that T be trivially copyable. So it seems like this should work:<br>
<br>
#include <atomic><br>
#include <type_traits><br>
<br>
struct A<br>
{<br>
int i_;<br>
<br>
A(int i) : i_(i) {}<br>
};<br>
<br>
static_assert(std::is_trivially_copyable<A>::value, "");<br>
<div class="im"><br>
int<br>
main()<br>
{<br>
std::atomic<A> q(A(1));<br>
}<br>
<br>
</div>The _Atomic type specifier was added to libc++ by this commit:<br>
<br>
---------------------------<br>
r146865 | theraven | 2011-12-19 06:44:20 -0500 (Mon, 19 Dec 2011) | 7 lines<br>
<br>
Some fixes to <atomic> operations to explicitly use atomic types and operations.<br>
<br>
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).<br>
<br>
Howard: Please review.<br>
---------------------------<br>
<br>
David Chisnall and I had a private conversation about this addition:<br>
<br>
<br>
On Dec 19, 2011, at 1:53 PM, David Chisnall <<a href="mailto:csdavec@swan.ac.uk">csdavec@swan.ac.uk</a>> wrote:<br>
<br>
> On 19 Dec 2011, at 18:33, Howard Hinnant wrote:<br>
<br>
>> 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).<br>
>><br>
>> Does clang have any documentation on _Atomic?<br>
><br>
> It's part of the C1x spec and is supported by clang in C++ mode as an extension.<br>
><br>
> 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.<br>
<br>
I still don't fully understand why we need _Atomic in the C++11 <atomic>. However whether we do or not, I don't really care. All I care about is that <atomic> works. <atomic> can't be implemented in straight C++. It requires compiler support. I need a clang expert to advise me on what the libc++ <atomic> should look like to just work.<br>
<br>
In Oct. 2010 I wrote this as a proposal for how it should work and had an <atomic> written to this spec:<br>
<br>
<a href="http://libcxx.llvm.org/atomic_design_a.html" target="_blank">http://libcxx.llvm.org/atomic_design_a.html</a><br>
<br>
In late 2011 David Chisnall did the clang work to get <atomic> to work and updated <atomic> accordingly. In Apr 2012 you switched <atomic> from __atomic_* builtins to __c11_atomic_* builtins.<br>
<br>
<shrug> The libc++ <atomic> 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.</blockquote>
<div><br></div><div>I think there are basically two long-term options and one short-term workaround:</div><div><br></div><div>1) Use T rather than _Atomic(T), manually ensure that std::atomic<T> has the right alignment, and use the __atomic_* builtins instead of the __c11_atomic_* builtins (this is the approach which libstdc++ takes), or</div>
<div>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.</div><div>
<br></div><div>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.</div>
</div>