Skip to content

Conversation

hjelmn
Copy link
Member

@hjelmn hjelmn commented Oct 5, 2016

Assembly workarounds for PGI.

hjelmn added 2 commits October 5, 2016 08:59
This commit contains the following changes:

 - There is a bug in the PGI 16.x betas for ppc64 that causes them to
   emit the incorrect instruction for loading 64-bit operands. If not
   cast to void * the operands are loaded with lwz (load word and
   zero) instead of ld. This does not affect optimized mode. The work
   around is to cast to void * and was implemented similar to a
   work-around for a xlc bug.

 - Actually implement 64-bit add/sub. These functions were missing and
   fell back to the less efficient compare-and-swap implementations.

Thanks to @PHHargrove for helping to track this down. With this update
the GCC inline assembly works as expected with pgi and ppc64.

Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
(cherry picked from commit a36bdfe)
The add_64, sub_64, and cmpset_64 atomics used "+m" (*addr) to
indicate the asm also writes the memory location. This is better than
using a memory clobber. PGI 16.9 introduced a bug that causes a
compiler failure on the "+m" constraint (input/output). It seems to
work with "=m" (output) which matches the 32-bit atomics.

Fixes open-mpi#2086

Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
(cherry picked from commit 2edc77b)
@jjhursey
Copy link
Member

jjhursey commented Oct 6, 2016

Similar failure on this PR as in PR #2177:

shell$ uname -m
ppc64le
shell$ pgcc --version

pgcc 16.9-beta linuxpower target on Linuxpower 
The Portland Group - PGI Compilers and Tools
Copyright (c) 2016, NVIDIA CORPORATION.  All rights reserved.
shell$ ./configure --without-x CC=pgcc CXX=pgc++ FC=pgfortran
...
shell$ make
...
make[2]: Entering directory `/tmp/jjh-core/ompi-test/ompi-v2.0.x-ppc/opal/tools/wrappers'
  CCLD     opal_wrapper
../../../opal/.libs/libopen-pal.so: undefined reference to `opal_atomic_sc_64'
../../../opal/.libs/libopen-pal.so: undefined reference to `opal_atomic_add_64'
../../../opal/.libs/libopen-pal.so: undefined reference to `opal_atomic_ll_64'
../../../opal/.libs/libopen-pal.so: undefined reference to `opal_atomic_swap_64'
make[2]: *** [opal_wrapper] Error 2
make[2]: Leaving directory `/tmp/jjh-core/ompi-test/ompi-v2.0.x-ppc/opal/tools/wrappers'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/tmp/jjh-core/ompi-test/ompi-v2.0.x-ppc/opal'
make: *** [all-recursive] Error 1

@hjelmn
Copy link
Member Author

hjelmn commented Oct 7, 2016

Opps. I know why it doesn't work. It isn't using the inline assembly and we do not have the appropriate asm file for this to work in 2.0.x. 2.1.x will re-enable inline assembly for PGI but it will be a hard sell for 2.0.x. Will have to talk with Jeff and Howard about that.

We disabled this support a long time ago. Probably safe to assume
whatever bug we were working around no longer exists.

Closes open-mpi#2044

Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
(cherry picked from commit 795833b)
@hjelmn
Copy link
Member Author

hjelmn commented Oct 7, 2016

@hppritcha Right now PGI on PPC does not work at all with 2.0.x. In order for it to work it looks like we will have to re-enable inline asm for PGI. @sjeaugey Is that ok for 2.0.x?

It may break Open MPI with PGI versions older than 10.8. I may be able to add work arounds for the bugs in versions from when inline assembly is supported to 10.8 as I have with PPC for 16.x. Gasnet has all the relevant bugs documented.

@cparrott73
Copy link

Hi @hjelmn - I work for NVIDIA/PGI and do testing of Open MPI on POWER here. As far as we're concerned, we would be fine with enabling inline assembly support for PGI on Open MPI. This would enable Open MPI to compile as written with PGI on POWER. Let me know if you need any additional information from us.

@jjhursey
Copy link
Member

jjhursey commented Oct 7, 2016

The updated branch builds and passes some basic testing (make check and ring_c).
Thanks!

I'll add my review approval, and I'll let the author decide if it needs to be updated based on the comment about older PGI compilers.

Copy link
Member

@jjhursey jjhursey left a comment

Choose a reason for hiding this comment

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

Approved based on testing

@jsquyres
Copy link
Member

@hppritcha Good to go.

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.

5 participants