Skip to content

opal/ppc atomics: Optimize the RMB(). #9217

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 17, 2021

Conversation

awlauria
Copy link
Contributor

Signed-off-by: Austen Lauria [email protected]

@jjhursey
Copy link
Member

This change of the RMB() from lwsync to isync was suggested by a member of our research team a long time ago. I don't recall the exact argument for it other than it is lighter-weight and safe for Open MPI due to how it uses the RMB() operation.

I think it is fine to go in given our experience with it. However, it would be good if someone else looked over this change that was familiar with the use of atomics in OMPI and commented.

Some related reading:

  • In PR atomics/powerpc: Fix WMB instruction #3661 we fixed the WMB() and had a brief exchange about the RMB(). At which time we decided not to change the instruction.
  • PowerISA (v3.0B) Memory Barrier instructions (Reference Book II 4.6.1, 4.6.3, B.2) and XL compiler doc
    • sync (hwsync): (PowerISA Book II 4.6.3) Heavyweight sync. Full memory barrier which prevents any reordering.
    • lwsync : (PowerISA Book II 4.6.3) Lightweight sync. Partial memory barrier which prevents (from my reading) reordering of Load-Load, Store-Store, and Load-Store instructions. Excludes Store-Load instructions across the barrier.
    • isync : (PowerISA Book II 4.6.1) Instruction barrier. Wait for all previous instructions to complete.
    • eieio : (PowerISA Book II 4.6.3) Controls order of storage references to I/O devices.

@gpaulsen gpaulsen merged commit 03d7386 into open-mpi:master Aug 17, 2021
@awlauria awlauria deleted the optimize_ppc_rmb branch August 19, 2021 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants