On 24/10/11 12:16, Helge Bahmann wrote:
On Monday 24 October 2011 12:41:53 Anthony Williams wrote:
On 17/10/11 00:45, Tim Blechmann wrote:
The review of Helge Bahmann's Boost.Atomic library starts today, October 17th 2011. It will end on October 26th.
Just a few quick comments about correctness.
* Operations build on top of compare_exchange (such as fetch_xor in gcc-x86.hpp) use memory_order_relaxed for all operations. Are you sure there are enough compiler memory barriers? Shouldn't you pass the supplied memory order through to the CAS call?
only the "failure order" of CAS is specified as memory_order_relaxed, while the specified order is passed through as "success order" -- since it is retried until it succeeds, it should be okay
OK.
* In gcc-x86.h, the platform_cmpxchg64_strong implementation assumes that T is an integral type, such that you can do (int)(desired>>32) to take the top 32 bits, and (int)desired to take the low 32 bits, which is essentially just the signed and unsigned variants of the 64-bit integer. This should therefore not be a template, but a plain inline function. cas64strong.hpp uses it incorrectly for pointers, but this specialization is unnecessary anyway --- this file is only relevant for 32-bit x86.
it specializes to signed/unsigned, but you are right that could be resolved by a cast instead of a template
Maybe. Casts are ugly, but a template with only two specializations (signed+unsigned variants of the same type) seems a bit overkill.
(in general I want to preserve type-safety such as "long != long long" even if both are 64 bit; maybe this is not worth it)
This code only applies to 32-bit x86, where long tends to be 32-bits. Anthony -- Author of C++ Concurrency in Action http://www.stdthread.co.uk/book/ just::thread C++0x thread library http://www.stdthread.co.uk Just Software Solutions Ltd http://www.justsoftwaresolutions.co.uk 15 Carrallack Mews, St Just, Cornwall, TR19 7UL, UK. Company No. 5478976