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

Cray OpenMP wrapper flag #1794

Merged
merged 6 commits into from Jun 9, 2016
Merged

Cray OpenMP wrapper flag #1794

merged 6 commits into from Jun 9, 2016

Conversation

gppezzi
Copy link
Contributor

@gppezzi gppezzi commented Jun 6, 2016

First attempt to fix the "-True" flag that is added when using openmp as TC option on Cray.
@boegel @pforai

@boegel boegel added this to the v2.9.0 milestone Jun 6, 2016
@boegel
Copy link
Member

boegel commented Jun 6, 2016

@gppezzi looks perfect to me... Did you test it? Any example easyconfigs where you have openmp enabled?

@gppezzi
Copy link
Contributor Author

gppezzi commented Jun 6, 2016

I've tested with GROMACS GPU but this is WIP, I will try with other simpler cases and let you know.

@boegel
Copy link
Member

boegel commented Jun 6, 2016

@gppezzi maybe try building STREAM with the Cray* toolchains to test this?

@pforai
Copy link
Contributor

pforai commented Jun 6, 2016

@gppezzi, @boegel

The OpenMP flags are sadly inconsistent between the wrappers for CCE, Intel, GNU and PGI:

OpenMP support:
   -homp        Enables OpenMP and links in OpenMP libraries when
            possible using CCE.
            (This is the default.)

   -hnoomp      Disables OpenMP and links in non-OpenMP libraries
            when using CCE.

   -fopenmp     Enables OpenMP and links in OpenMP libraries when
            possible using GNU.

   -openmp      Enables OpenMP and links in OpenMP libraries when
            possible when using Intel.

   -mp          Enables OpenMP and links in OpenMP libraries when
            possible using PGI.

   -Mnoopenmp       Disables OpenMP and links in non-OpenMP libraries
            when using PGI.

@gppezzi
Copy link
Contributor Author

gppezzi commented Jun 6, 2016

If the new implementation makes sense to you, I'll apply the same fix accordingly to Intel, CCE and PGI. Let me know.

eb ~/GROMACS-5.1.2-CrayGNU-2016.03-cuda-7.0.eb -x | grep CFLAGS
  • Before the fix
  export CFLAGS="-craype-verbose -True -O2"
  export FCFLAGS="-craype-verbose -True -O2"
  • After
  export CFLAGS="-craype-verbose -fopenmp -O2"
  export FCFLAGS="-craype-verbose -fopenmp -O2"

@pforai
Copy link
Contributor

pforai commented Jun 6, 2016

Yeah, this looks good!

Update the unique compiler flags map for each of the subclasses accordingly.

@gppezzi
Copy link
Contributor Author

gppezzi commented Jun 6, 2016

I've updated the other subclasses.

So far I've tested this for GNU and Intel. CCE and PGI still need to be checked.

@@ -38,3 +38,4 @@ class CrayIntel(CrayPEIntel, CrayMPICH, LibSci):
"""Compiler toolchain for Cray Programming Environment for Intel compilers (PrgEnv-intel)."""
NAME = 'CrayIntel'
SUBTOOLCHAIN = DUMMY_TOOLCHAIN_NAME
CrayPEIntel.COMPILER_UNIQUE_OPTION_MAP.update({'openmp': 'openmp'})
Copy link
Member

Choose a reason for hiding this comment

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

openmp is deprecated for recent Intel versions, better stick to fopenmp here too, cfr. #1718

@boegel
Copy link
Member

boegel commented Jun 7, 2016

@gppezzi I think you'll need to tackle this in easybuild/toolchains/compiler/craype.py instead, where we already fiddle with self.COMPILER_UNIQUE_OPTION_MAP for the precision flags (which are also compiler specific)

@gppezzi
Copy link
Contributor Author

gppezzi commented Jun 7, 2016

@boegel now tested with CCE, Intel and GNU (I don't have any test for PGI)
cc @pforai @jgphpc

  • CCE
export CFLAGS="-craype-verbose -homp -O2"
  • Intel
export CFLAGS="-craype-verbose -fopenmp -O2 -ftz -fp-speculation=safe -fp-model"
  • GNU
export CFLAGS="-craype-verbose -fopenmp -O2"

@@ -137,6 +137,7 @@ class CrayPEGCC(CrayPECompiler):
def __init__(self, *args, **kwargs):
"""CrayPEGCC constructor."""
super(CrayPEGCC, self).__init__(*args, **kwargs)
self.COMPILER_UNIQUE_OPTION_MAP.update({'openmp': 'fopenmp'})
Copy link
Member

Choose a reason for hiding this comment

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

@gppezzi I would go with this instead, just to match the style below:

self.COMPILER_UNIQUE_OPTION_MAP['openmp'] = 'fopenmp'

other than that: should be good to go

@gppezzi
Copy link
Contributor Author

gppezzi commented Jun 9, 2016

@boegel any other remark?

@boegel
Copy link
Member

boegel commented Jun 9, 2016

lgtm, thanks @gppezzi

@boegel boegel merged commit 8c4e0bb into easybuilders:develop Jun 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants