Re: [boost] [Boost-users] [Review] Boost.Atomic review starts today, October 17th 2011

Subject: Re: [boost] [Boost-users] [Review] Boost.Atomic review starts today, October 17th 2011
From: Anthony Williams (anthony.ajw_at_[hidden])
Date: 2011-10-24 07:30:22

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