Boost C++ Libraries: Ticket #9426: boost::atomic gcc-cas related specializations patch

attachment set

Sat, 23 Nov 2013 01:14:17 GMT

attachment → boost_1_55_0_atomic_gcc_cas.patch

timblechmann — Sat, 23 Nov 2013 11:48:59 GMT

unfortunately the original author disappeared and i have no idea what's the problem with the original code. could be a compiler bug which may or may not have been fixed ... in general, the patch looks ok to me, though...

Andrey Semashev — Sat, 23 Nov 2013 14:55:02 GMT

I welcome the compilation fixes for the gcc-cas.hpp and cas32strong.hpp headers but I don't see the point in the configuration macro. The ARM target should use the implementation from gcc-armv6plus.hpp (not lock-based). If it doesn't then that's probably a bug in the target detection in platform.hpp and I'd prefer it to be fixed instead. gcc-cas.hpp should be included only since gcc 4.1, because __sync* intrinsics were not available before. It should also check if __GCC_HAVE_SYNC_COMPARE_AND_SWAP_N macros are defined. I suppose, that implementation would be used in case of MIPS. In any case, there is no need in forcing gcc-cas.hpp, unless I'm missing something.

PS: I'm not aware of any hidden problems with gcc-cas.hpp. When it's compilation problems are fixed I'm in favor of enabling it in platform.hpp.

cc set

Andrey Semashev — Sat, 23 Nov 2013 14:55:52 GMT

cc Andrey.Semashev@… added

timblechmann — Sat, 23 Nov 2013 15:14:40 GMT

iirc the __GCC_HAVE_SYNC macros have been introduced with a later gcc version, so it is probably a good idea only to use the gcc-cas version when these macros are available.

regaring arm, it may actually be a good idea to use gcc-cas on armv7, because it provides a double-width ll/sc, which is currently not supported by boost.atomic (but quite useful for many lock-free algorithms). or implement the complete arm support in assembly to avoid the ll/sc emulating cas emulating atomics issue ...

Andrey Semashev — Sat, 23 Nov 2013 16:26:14 GMT

or implement the complete arm support in assembly to avoid the ll/sc emulating cas emulating atomics issue

That was my idea, yes.

Sat, 23 Nov 2013 18:12:47 GMT

Just to make it clear, my patch was not ARM targeted (nor any other possible target that uses default atomics impl. currently) -- I just used ARM to verify gcc-cas implementation correctness. I guess that current selection for ARMs is usually better than gcc-cas (at least for ARMs 6+). My primary intent was to allow use of gcc-cas on MIPSel platforms on which pthread mutexes are currently being selected by default. BTW, I'd prefer to use lower level APIs for that, but currently I can not use nothing better then generic atomics for Mips implemented based on strong memory barrier only -- which most likely gives no noticeable advantages over implementation from gcc-cas.

Regarding optional use of gcc-cas, if you think, that unconditional use of it (besides of earlier checks from platform.hpp and additional check for minimum gcc version supporting __sync API of course) is safe enough and better than default selection, I'd also opt for that.

tim@… — Sat, 23 Nov 2013 18:23:02 GMT

piotr: not being familiar with mipsel toolchains, i wonder: what (gcc) compilers are commonly used for this architecture? e.g. is it common to use gcc older than 4.4? iirc 4.3 or 4.4 started to define the __GCC_HAVE_SYNC macros ... and tbo i'd rather not spend too much time on toolchains which are hardly used these days (e.g. gcc-3.X) or the like ...

andrey: real native arm support would be great, but also here i wonder: is it really worth to spend time on this? i suppose that most arm toolchains provide std::atomic these days...

Andrey Semashev — Sat, 23 Nov 2013 20:17:27 GMT

real native arm support would be great, but also here i wonder: is it really worth to spend time on this? i suppose that most arm toolchains provide std::atomic these days...

Well, I'd like Boost.Atomic to be at least not worse than std::atomic on all supported targets. If Boost.Atomic offers additional features, such as specialized operations, this may be a good reason to prefer it over std::atomic.

Sun, 24 Nov 2013 00:29:31 GMT

not being familiar with mipsel toolchains, i wonder: what (gcc) compilers are commonly used for this architecture?

Hard to tell precisely, since it depends on strategy of the processor vendors adopted to theirs individual customers (which, in general, I'm not too aware of). But I'm afraid, that some people can still use toolchains for Mips older than 4.1 (which I think is initial version supporting __sync_synchronize builtin). Nevertheless, I agree that having just gcc version check would most likely be enough here -- anyone interested in using of older toolchains will still safely use default alternative.

I'm just not sure, whether suggested use of __GCC_HAVE_SYNC_* macros shall be preferred before simple gcc version check -- official documentation for gcc 4.1 does not list it in predefined macros list, thus I'd rather prefer use of version check.

attachment set

Sun, 24 Nov 2013 17:47:30 GMT

attachment → boost_1_55_0_atomic_gcc_cas_no_opt.patch

Sun, 24 Nov 2013 17:48:48 GMT

Added patch that checks gcc version (4.1+) instead of use of BOOST_ATOMIC_ALLOW_GCC_CAS option before inclusion of gcc-cas.hpp.

status changed; resolution set

timblechmann — Sat, 07 Dec 2013 09:49:16 GMT

status new → closed
resolution → fixed

applied the patch to git