Update opal to use C11 atomics if available

[hjelmn]

This PR introduces a new set of types to be used for all opal atomics (opal_atomic_*) and adds support for C11 atomics. If C11 atomics are detected they are used exclusively.

This PR should be safe for v4.0.0 if we want to merge it today. I have no opinion either way.

[hjelmn]

@bosilca Can you take a look?

@jsquyres @bwbarrett @hppritcha It is my opinion we should always use the language support if it is available. I am sure there will be disagreement so I would like to know how we should proceed. I cab make configure prefer C11 atomics and give a way to force either sync builtins or the hand-written atomics.

[hjelmn]

Needs some more work. Will update later today.

[gpaulsen]

Would be great for v4.0, but don't hold v4.0 branch.

[hjelmn]

Think I fixed everything. The opal_atomic_*_ptr functions were changed to work on intptr_t instead of void *. This is due to the fact that in C11 there is no atomic_void *. We could always define one but atomic_intptr_t is the recommended type. Didn't quite get everything updated correctly on the first go.

[hjelmn]

No user-facing things changed. All this is completely internal.

[hjelmn]

As for downsides, I don't think there are any vs what we already have with the gcc/icc builtins (__sync, __atomic). In fact, __atomic is likely the underlying implementation of the C11/C++11 atomics.

I plan to run some performance tests today to make sure there is no slowdown with the C11 atomics. I do not expect one but need to be sure.

[hjelmn]

Ok, that should make the pull request build checker happy. Forgot the int128_t C11 atomics on x86_64 with gcc are not lock-free. The workaround is to use __sync builtins for the 128-bit CAS.

[hjelmn]

Performance of RMA-MT on Cray XC shows no performance difference between C11 and the hand-written assembly. That is a good sign :).

[hjelmn]

@hoopoepg The ARM64 tests failed for something unrelated to this PR. Looks like it is in oshmem. Can you take a look at the errors in https://jenkins.open-mpi.org/jenkins/job/open-mpi.build.platforms/3283/Platform=ARMv8/

[hjelmn]

Ok, looks good to go now. The only failure appears to be unrelated to this PR.

[hjelmn]

Hmm, i see what is going wrong:

base/atomic_base_available.c: In function ‘init_query’:
base/atomic_base_available.c:107:23: error: expected identifier before ‘__extension__’
         ret = atomic->atomic_init(enable_progress_threads, enable_threads);

@hoopoepg atomic_init is a C-language macro. You need to change the name.

In fact, you need to avoid using any of these identifiers: https://en.cppreference.com/w/c/atomic . They are not reserved but at some point we will require C11 and will always include stdatomic.h.

[hoopoepg]

@hjelmn #5473 will work?

[hjelmn]

@hoopoepg perfect. thanks!

[hjelmn]

:bot:retest

[hjelmn]

Ok, so far so good. I have to address one bug then this is good to go. Turns out the implicit memory order is memory_order_seq_cst. We want many of the atomics to be memory_order_relaxed. Will fix and push the update.

[hjelmn]

@bosilca Any comments on this?

[bosilca]

I only have positive comments. Maybe the only concern I have with this patch is the overwhelming number of functions for atomic support with most of them never used and doubtfully needed in the future. I might have preferred a smaller patch, one that only provide the _strong version of the atomic functions (which is the only one we currently use if I'm not mistaken).