Skip to content
This repository was archived by the owner on Sep 30, 2022. It is now read-only.

Conversation

@hjelmn
Copy link
Member

@hjelmn hjelmn commented Aug 30, 2016

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/ompi#1854

Signed-off-by: Nathan Hjelm hjelmn@lanl.gov

(cherry picked from commit open-mpi/ompi@a9bc692)

Signed-off-by: Nathan Hjelm hjelmn@lanl.gov

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/ompi#1854

Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>

(cherry picked from commit open-mpi/ompi@a9bc692)

Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
@hjelmn
Copy link
Member Author

hjelmn commented Aug 30, 2016

:bot:assign: @jjhursey
:bot🏷️bug
:bot:milestone:v2.0.2

@jsquyres This seems to fix the issue @PHHargrove reported when using xlc

@ompiteam-bot ompiteam-bot added this to the v2.0.2 milestone Aug 30, 2016
@mellanox-github
Copy link

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ompi-release-pr/2136/ for details.

@jjhursey
Copy link
Member

👍

I ran a test with and without this patch. Without this patch it fails in make check on dlopen_test. With this patch it passes that and a ring_c MPI program.

@PHHargrove
Copy link
Member

Josh's findings are consistent with my own on 3 distinct systems.
So, I second the +1.
-Paul

@hppritcha
Copy link
Member

@jsquyres I think this is ready to merge.

@jsquyres jsquyres merged commit 353e178 into open-mpi:v2.x Sep 12, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dlopen_test crash with xlc on PPC in v2.x

7 participants