Skip to content
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

Fix patcher for ppc and aarch64 #13027

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bosilca
Copy link
Member

@bosilca bosilca commented Jan 9, 2025

Prevent the compiler from reordering the instructions in the patcher by marking the assembly code as clobbering memory.

Thanks @pinskia for the patch and for bringing the discussion on the gcc bugzilla.

I was not able to test this. The code generated by godbolt seems legit, but I did not yet figured out how to look at the disassembled code in OMPI.

Fixes #13014.

Prevent the compiler from reordering the instructions in the patcher by
marking the assembly code as clobbering memory.

Thanks @pinskia for the patch and for bringing the discussion on the gcc
bugzilla.

Fixes open-mpi#13014.

Signed-off-by: George Bosilca <[email protected]>
@bosilca bosilca added the bug label Jan 9, 2025
@bosilca bosilca added this to the v5.1.0 milestone Jan 9, 2025
@bosilca bosilca requested a review from janjust January 9, 2025 08:07
asm volatile("std 2, %0" : "=m"(toc_save)); \
asm volatile("nop; nop; nop; nop; nop");
# define OPAL_PATCHER_END asm volatile("ld 2, %0" : : "m"(toc_save));
asm volatile("std 2, %0" : "=m"(toc_save) :: "memory"); \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a reference to the GCC ticket to remember why we globber the memory here? Just in case someone looks at this in 3 years from now and tries to save some cycles...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

OPAL_PATCHER_BEGIN/OPAL_PATCHER_END is broken in some cases
3 participants