On Mon, May 20, 2013 at 2:45 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</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">On Mon, May 20, 2013 at 2:36 PM, Howard Hinnant <span dir="ltr"><<a href="mailto:hhinnant@apple.com" target="_blank">hhinnant@apple.com</a>></span> wrote:<br></div></div><div class="gmail_quote">
<div><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div>On May 20, 2013, at 5:20 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:<br>
<br>
> On Mon, May 20, 2013 at 2:07 PM, Howard Hinnant <<a href="mailto:hhinnant@apple.com" target="_blank">hhinnant@apple.com</a>> wrote:<br>
><br>
> On May 20, 2013, at 4:44 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">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" target="_blank">hhinnant@apple.com</a>> wrote:<br>
> > On May 20, 2013, at 3:29 PM, Rich E <<a href="mailto:reakinator@gmail.com" target="_blank">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>
> 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>
><br>
> int<br>
> main()<br>
> {<br>
> std::atomic<A> q(A(1));<br>
> }<br>
><br>
> 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" target="_blank">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.<br>
><br>
> I think there are basically two long-term options and one short-term workaround:<br>
><br>
> 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<br>
> 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.<br>
><br>
> 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.<br>
<br>
</div></div>Thanks. Do we have any documentation for the __atomic_* builtins?<br></blockquote><div><br></div></div></div><div>They're documented in the GCC manual:</div><div><br></div><div><a href="http://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html" target="_blank">http://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html</a></div>
</div></blockquote><div><br></div><div>One thing to be cautious about:</div><div><br></div><div>struct T { char c[3]; };</div><div>static_assert(sizeof(std::atomic<T>) == 3 && alignof(std::atomic<T>) == 1, "under libstdc++");</div>
<div>static_assert(sizeof(std::atomic<T>) == 4 && alignof(std::atomic<T>) == 4, "under libc++");</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_quote"><div> </div><div class="im"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
In file included from test.cpp:1:<br>
/Users/hhinnant/Development/temp_libcxx/include/atomic:564:50: error: too few arguments to function call, expected 2, have 1<br>
{return __atomic_is_lock_free(sizeof(_Tp));}<br>
~~~~~~~~~~~~~~~~~~~~~ ^<br>
/Users/hhinnant/Development/temp_libcxx/include/atomic:567:50: error: too few arguments to function call, expected 2, have 1<br>
{return __atomic_is_lock_free(sizeof(_Tp));}<br>
~~~~~~~~~~~~~~~~~~~~~ ^<br>
/Users/hhinnant/Development/temp_libcxx/include/atomic:1362:51: error: too few arguments to function call, expected 4, have 3<br>
{return __atomic_exchange(&__a_, true, __m);}<br>
~~~~~~~~~~~~~~~~~ ^<br>
/Users/hhinnant/Development/temp_libcxx/include/atomic:1365:51: error: too few arguments to function call, expected 4, have 3<br>
{return __atomic_exchange(&__a_, true, __m);}<br>
~~~~~~~~~~~~~~~~~ ^<br>
4 errors generated.<br>
<span><font color="#888888"><br>
<br>
Howard<br>
<br>
</font></span></blockquote></div></div><br>
</blockquote></div><br>