
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.