-
Notifications
You must be signed in to change notification settings - Fork 937
opal/patcher: fix xlc support #2021
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
Conversation
The xlc compiler seems to behave in a different way that gcc when it
comes the inline asm. There were two problems with the code with xlc:
- The TOC read in mca_patcher_base_patch_hook used the syntax
register unsigned long toc asm("r2") to read $r2 (the TOC
pointer). With gcc this seems to behave as expected but with xlc
the result in toc is not the same as $r2. I updated the code to use
asm volatile ("std 2, %0" : "=m" (toc)) to load the TOC pointer.
- The OPAL_PATCHER_BEGIN macro is meant to be the first thing in a
hook. On PPC64 it loads the correct TOC pointer (thanks to
mca_patcher_base_patch_hook) and saves the old one. The
OPAL_PATCHER_END macro restores the TOC pointer. Because we *need*
the TOC to be correct before it is accessed in the hook the
OPAL_PATCHER_BEGIN macro MUST come first. We did this and all was
well with gcc. With xlc on the other hand there was a TOC access
before the assembly inserted by OPAL_PATCHER_BEGIN. To fix this
quickly I broke each hook into a pair of function with the
OPAL_PATCHER_* macros on the top level functions. This works around
the issue but is not a clean way to fix this. In the future we
should 1) either update overwrite to not need this, or 2) figure
out why xlc is not inserting the asm before the first TOC read.
This fixes open-mpi#1854
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
|
@jjhursey Please double-check. This is running cleanly on my Power8 system with xlc with both --enable-static and without. At some point we should probably get together with the xlc team and see why the assembly wasn't working as expected. |
|
@jsquyres Ideally this should be in 2.0.1 but IBM could use this in their own internal version until we get out 2.0.2. |
|
I'll try to test on Monday. |
|
Merging to let MTT have a crack at it. |
|
I will also try to retest at least three distinct ppc64 platforms this weekend if I have sufficient net access. |
|
My testing consisted of building the most recent master tarball (openmpi-dev-4711-gd33204b), including:
The following two both passed all of those steps, while the 2.0.1rc1 tarball got a SEGV in dlopen_test from "make check" (and thus didn't get as far as a ring_c test which the following ran fine): Power8 little-endian Power7 big-endian The following had not tested 2.0.1rc1 previously (but when tested today failed in a DIFFERENT way to be reported soon), but passed on master: Power7 big-endian |
When 2.0.1rc is configured with |
|
I just tested I also tested the current Both configured the same way: |
|
@jjhursey I couldn't reproduce the issue with make check. I was able to reproduce using a simple MPI application (ring_c). |
|
@PHHargrove Thanks for testing! Looks like the fix worked. |
The xlc compiler seems to behave in a different way that gcc when it
comes the inline asm. There were two problems with the code with xlc:
register unsigned long toc asm("r2") to read $r2 (the TOC
pointer). With gcc this seems to behave as expected but with xlc
the result in toc is not the same as $r2. I updated the code to use
asm volatile ("std 2, %0" : "=m" (toc)) to load the TOC pointer.
hook. On PPC64 it loads the correct TOC pointer (thanks to
mca_patcher_base_patch_hook) and saves the old one. The
OPAL_PATCHER_END macro restores the TOC pointer. Because we need
the TOC to be correct before it is accessed in the hook the
OPAL_PATCHER_BEGIN macro MUST come first. We did this and all was
well with gcc. With xlc on the other hand there was a TOC access
before the assembly inserted by OPAL_PATCHER_BEGIN. To fix this
quickly I broke each hook into a pair of function with the
OPAL_PATCHER_* macros on the top level functions. This works around
the issue but is not a clean way to fix this. In the future we
should 1) either update overwrite to not need this, or 2) figure
out why xlc is not inserting the asm before the first TOC read.
This fixes #1854
Signed-off-by: Nathan Hjelm hjelmn@lanl.gov