Boost logo

Boost :

Subject: Re: [boost] [release/atomic] request permission to merge 86144
From: Gavin Lambert (gavinl_at_[hidden])
Date: 2013-10-14 18:51:15

On 15/10/2013 03:07, Quoth Andrey Semashev:
> I don't think the commit is correct.

I was the original author of the patch, and I can confirm that it is
correct at least in the fact that DCAS (and other 64-bit atomic
operations) did not work on VS2008 prior to the patch and does after
applying it.

I did not test on other platforms or compilers but by inspection the
changes in the patch should be skipped or otherwise harmless in those cases.

> 1. 64-bit CAS for 32-bit x86 was already supported before. It is controlled by
> the BOOST_ATOMIC_X86_HAS_CMPXCHG8B macro, which is automatically defined when
> the compiler targets Pentium or later (may need to be enabled by a compiler
> switch).

The patch was originally based on Boost 1.53 and 1.54, in which this
macro is not defined at all.

It does look like better support for 64-bit compare_exchange_* and
load/store specifically has been added to trunk (and presumably 1.55)
since then though.

But the patch should still add support for fetch_add, fetch_sub, and
exchange operations, which still appear to be otherwise unsupported in
trunk prior to the patch as far as I can tell.

> 2. Your change modifies 64-bit branch, which always have 64-bit atomic ops
> available (i.e. CAS-based path is not needed). This branch is not used by 32-
> bit targets.

No, it's in the #else clause of the 64-bit branch, aka the 32-bit branch.

Probably the changes in interlocked.hpp should now also be wrapped in a
test for BOOST_ATOMIC_X86_HAS_CMPXCHG8B, since you're right that this
would be more correct.

But the intrinsic *does* exist on x86 >= Pentium, so defining it here
means that the assembly code in platform_cmpxchg64_strong is not
required. (Having said that, it would probably be harmless to
completely omit the changes to interlocked.hpp.)

The changes in windows.hpp still seem relevant as noted above.

Boost list run by bdawes at, gregod at, cpdaniel at, john at