Re: [boost] Patch to boost/detail/interlocked.hpp

From: Anthony Williams (anthony_w.geo_at_[hidden])
Date: 2005-09-23 12:23:59

"Peter Dimov" <pdimov_at_[hidden]> writes:

> Anthony Williams wrote:
>>> I am trying to change my call_once implementation for boost.thread,
>>> so it doesn't require windows.h to be included. In doing so, I tried
>>> to use boost/detail/interlocked.hpp, and discovered that it doesn't
>>> work on gcc-mingw-4.0.1, since the imported functions are declared
>>> in boost::detail namespace, and the macros don't qualify the names.
>>>
>>> Attached is a patch for the file that changes the macros to refer to
>>> the imported functions by fully qualified names.
>>>
>>> Please can someone check and commit this for me.
>
> Apologies for not being able to respond earlier.
>
> I have some (very) minor comments about your changes. Qualifying 
> _Interlocked* with :: appears to work as intended - although I haven't 
> tested it extensively - but looks very odd for someone reading the code; 
> these identifiers are reserved and there shouldn't be any namespace clashes, 
> so it might be best to leave them unqualified.

OK. Changed.

> BOOST_INTERLOCKED_READ doesn't really belong in interlocked.hpp (macro vs 
> inline aside). The aim of this header is only to provide the Interlocked* 
> functions as specified and documented by Microsoft without including 
> <windows.h>; it is not meant to introduce new unspecified and undocumented 
> functionality.

Fair enough. I'll move them elsewhere. I used macros rather than inline
functions, for consistency with the rest of the INTERLOCKED stuff. Maybe
inline functions are more appropriate, since these are users of the
INTERLOCKED functions rather than direct mappings.

Moved to boost/thread/detail/interlocked_read_win32.hpp.

> Finally, I believe that for correct double-checked locking you only need a 
> load with acquire barrier on the fast path - which maps to an ordinary load 
> on x86(-64) and to ld.acq on IA-64 - and by using a fully locked cmpxchg 
> you're introducing a performance penalty (the philosophical debate of 
> whether InterlockedCompareExchange is guaranteed to enforce memory ordering 
> when the comparison fails aside.) 

Is there an intrinsic function for that? I couldn't find one, which is why I
left it at InterlockedCompareExchange. I guess it could use
InterlockedCompareExchangeAcquire, which reduces the locking penalty.

Anthony
-- 
Anthony Williams
Software Developer
Just Software Solutions Ltd
http://www.justsoftwaresolutions.co.uk