On 15/10/2013 13:06, Quoth Andrey Semashev:
However, I would appreciate if you tried out the current trunk sans the commit 86144 to verify it works.
That could be awkward for me at present, but I'll see if I can try the 1.55 beta build, which should have the same effect as that commit hasn't been merged through yet.
No, the trunk code before 86144 had support for all 64-bit operations, both for 64 and 32-bit targets. In the latter case, they were implemented through DCAS (see atomic/detail/cas64strong.hpp).
Ah yes, I see now. I don't have the trunk code downloaded at the moment so I'm looking at it through an SVN browser peephole, and it's easy to miss details in other files.
Are we looking at the same code? The change to windows.hpp starts at line 1313, which is inside the "#if defined(_M_AMD64) || defined(_M_IA64)" block. The rest of the changes are also within this block. I'm looking at the current trunk now.
Sorry, I thought you were referring to the interlocked.hpp changes. You're right, it looks like the windows.hpp changes have been misapplied, due to the changes between 1.54 and 1.55/trunk. (They're not *completely* useless, as it still defines a fallback path in case eg. BOOST_ATOMIC_INTERLOCKED_EXCHANGE_ADD64 is not defined on x64 for some reason, but as currently that can't happen it's a needless redundancy.)
That macro is not defined when interlocked.hpp is included. The change to interlocked.hpp is not needed because windows.hpp doesn't use the _InterlockedCompareExchange64 intrinsic to implement cmpxchg8b.
I noticed that, although I'm not entirely sure why it doesn't. Wouldn't using the intrinsic be simpler than using the assembly code directly? It probably doesn't really matter in the end though, the effect should be the same.
Given the above, I don't think so. I'm in favor of reverting that commit.
I agree with that now. I was not aware of the 1.55 changes at the time I made the patch.