Skip to content
New issue

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

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

Already on GitHub? # 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 <gbosilca@nvidia.com>
@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.

# for free to join this conversation on GitHub. Already have an account? # 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