ompi
ompi copied to clipboard
C11 Atomics: add relaxed load and store to avoid superfluous atomic instructions
The C11 _Atomic
qualifier requires accesses to such variables to be sequentially consistent [1], leading to unnecessary atomic operations in code paths where there cannot be concurrent accesses by other threads (e.g., initialization of data structures such as opal_object_t
and the non-atomic fallbacks in the opal_thread_*
functions). In fact, the whole shabeng about avoiding atomics in opal_thread_*
calls is moot with C11.
This PR introduces OPAL_ATOMIC_RELAXED_STORE
and OPAL_ATOMIC_RELAXED_LOAD
to avoid atomic
operations in paths where atomicity and memory ordering is not required. On my machine, a sequence of irecv - send - wait
takes 0.22us on current main and 0.16us with this patch. I'm sure there are plenty of other places where we should use OPAL_ATOMIC_RELAXED_STORE
instead of simple assignment but finding them is tedious.
Alternatively, we may consider removing _Atomic
from the opal_atomic_*
types and use volatile
instead (to make sure polling on variables is not optimized out). That way, we don't have to litter ugly macros like OPAL_ATOMIC_RELAXED_STORE
throughout the code and keep the C11 atomic backend consistent with the other backends. Plus, we won't run the chance that there are stray atomic operations that were not caught. After all, all code in Open MPI relies on the opal_atomic*
functions to perform atomic operations and direct accesses to atomic variable are not expected to be atomic or ordered. I wasn't around when the decision was made to use _Atomic
so I don't know what the arguments were back then though.
For the record: a simple atomic load/store interface for opal atomic was requested in https://github.com/open-mpi/ompi/issues/9722, which is what this PR adds.
[1] https://en.cppreference.com/w/c/language/atomic
Signed-off-by: Joseph Schuchart [email protected]
The IBM CI (PGI) build failed! Please review the log, linked below.
Gist: https://gist.github.com/a8b3319abf9c97d5d0ac7d1f57c3eac9
The IBM CI (GNU/Scale) build failed! Please review the log, linked below.
Gist: https://gist.github.com/7d3cdb42f2e690a6ac5e84c2a30bcf77
The IBM CI (XL) build failed! Please review the log, linked below.
Gist: https://gist.github.com/a9ec65e552ace7c62d57961517834ba9
Closing in favor of https://github.com/open-mpi/ompi/pull/10613