openmp link flags for compilers

[davydden]

The need to provide openmp link flags came up in a couple of place, see #875 or #845 .

Different compilers need different flags (see https://www.dartmouth.edu/~rc/classes/intro_openmp/compile_run.html ):

-- GNU (gcc, g++, gfortran) : -fopenmp
-- Intel (icc ifort): -openmp
-- Portland Group (pgcc,pgCC,pgf77,pgf90) : -mp

As far as i can tell, the flags are the same for C/C++ and Fortran. So we don't need to discriminate between those cases.

Obviously, this must be related to Compiler class, and those flags are to be implemented in $SPACK_ROOT/lib/spack/spack/compilers/clang.py and alike.

(1) First approach, example for gcc

    @classmethod
    def openmp_link_flags(self):
        return '-fopenmp'

and for clang

    @classmethod
    def openmp_link_flags(self):
        # test that this is Apple's clang and then...
        raise InstallError('Clang does not support OpenMP!')
        return ' '

Then inside the package.py it would be used where needed as (if I am not mistaken)

self.compiler.openmp_link_flags

(2) Second approach: I don't know if this is possible, but a more flexible alternative (in the spirit of #657) would be to have in compiler class

def setup_dependent_package(self, module, dspec):
   if `+openmp` in dspec
      self.spec.openmp_link_flags = '-fopenmp`

Shall we need to add more dynamic checks based on specs, it would be much easier with (2) approach, as far as i understand the whole framework.

Either of this should be very straightforward to add, and I will be happy to give it a go if we agree on the approach.

@tgamblin @mathstuf @eschnett @adamjstewart @alalazo @citibeth : what do you think?

[citibeth]

I don't like either way. We can't have our compiler classes depending on
specific packages (such as OpenMP).

Is there a general mechanism that could be USED to provide the desired
behavior with OpenMP, but does not reference openmp (or any other package)
in the core Spack code? We then make sure that Spack supports this
meachanism.

Maybe the recent work on appropriate BLAS/LAPACK flags is the right
direction to go. See #657.

On Sun, May 1, 2016 at 2:20 PM, Denis Davydov notifications@github.com
wrote:

The need to provide openmp link flags came up in a couple of place, see
#875 #875 or #845
#845 .

Different compilers need different flags (see
https://www.dartmouth.edu/~rc/classes/intro_openmp/compile_run.html ):

-- GNU (gcc, g++, gfortran) : -fopenmp
-- Intel (icc ifort): -openmp
-- Portland Group (pgcc,pgCC,pgf77,pgf90) : -mp

As far as i can tell, the flags are the same for C/C++ and Fortran. So we
don't need to discriminate between those cases.

Obviously, this must be related to Compiler class, and those flags are to
be implemented in $SPACK_ROOT/lib/spack/spack/compilers/clang.py and
alike.

(1) First approach, example for gcc

@classmethod
def openmp_link_flags(self):
    return '-fopenmp'

and for clang

@classmethod
def openmp_link_flags(self):
    # test that this is Apple's clang and then...
    raise InstallError('Clang does not support OpenMP!')
    return ' '

Then inside the package.py it would be used where needed as (if I am not
mistaken)

self.compiler.openmp_link_flags

(2) Second approach: I don't know if this is possible, but a more flexible
alternative would be to have in compiler class

def setup_dependent_package(self, module, dspec):
if +openmp in dspec
self.spec.openmp_link_flags = '-fopenmp`

shall we need to make add more dynamic checks based on specs, it would be
much easier with (2) approach, as far as i understand the whole framework.

Either of this should be very straightforward to add, and I will be happy
to give it a go if we agree on the approach.

@tgamblin https://github.com/tgamblin @mathstuf
https://github.com/mathstuf @eschnett https://github.com/eschnett
@adamjstewart https://github.com/adamjstewart @alalazo
https://github.com/alalazo @citibeth https://github.com/citibeth :
what do you think?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#876

[davydden]

@citibeth :

We can't have our compiler classes depending on
specific packages (such as OpenMP).

I have a feeling that you misunderstand what openmp is. It is not a package. It is an API which (from http://openmp.org/)

supports multi-platform parallel programming in C/C++ and Fortran.

with shared memory. State of affairs is that compilers have to implement it if they want to allow users to utilize this. It is absolutely opposite to mpi which is a library to do parallel computation with distributed memory.

[davydden]

@citibeth: FYI openmp is also different from TBB which is again a library/package for parallel programming with shared memory.

[adamjstewart]

Haha, I was already working on this actually, but my solution was much simpler. I just added an openmp_flag variable to every compiler. Here is the problem I see with your approach. As we've discussed in other PR's, there are various reasons for wanting both an openmp and non-openmp version of a package. We can't just always build with openmp when the compiler supports it. Since we'll always need an openmp variant anyway, I think we should leave it to the user to choose the right variant for their compiler. If openmp is always off by default, I don't think many users would explicitly enable it without knowing what it was and whether or not their compiler supports it. Also, future versions of Clang will support openmp anyway.

When a user builds a package with clang and explicitly enables openmp, does the config.log shows a useful error message? If so, that would make my argument more defendable. If not, then perhaps it would be worthwhile to turn this flag into a function like you suggested, but have it depend on the version of the compiler. I would also still argue that it's worthwhile to split Clang and Apple/LLVM into two separate compilers. I'm willing to do the leg work for that depending on how people feel about that suggestion.

By the way, here is a more complete table of openmp flags per compiler:

Compiler	OpenMP Flag
GCC	-fopenmp
Clang	-fopenmp
Intel	-openmp
NAG	-openmp
PGI	-mp
XL	-qsmp=omp

[davydden]

there are various reasons for wanting both an openmp and non-openmp version of a package. We can't just always build with openmp when the compiler supports it

i fully agree. Perhaps i was not clear above, i did not intend to always add openmp flags.

but have it depend on the version of the compiler.

agree again! 😄 I don't know the internals, but with both approaches suggested above i assumed we could query compiler version and do (for example for clang) something like

  ver = '%s' % self.compiler.version;
  if ver.endswith('-apple'):
     raise Error('You are out of luck. Apple don't care about Openmp for now')
  else:
     return `-fopenmp`

split Clang and Apple/LLVM into two separate compilers

That is an option. I tend to agree that this is the way to go. Alternatively, with less work one could add extra checks (like the one above) to see if clang is from Apple or not.

[adamjstewart]

Personally, I would love to make compilers a build dependency. Then, that compiler can provides('openmp', when='@version:'), but I remember there being significant push-back to that approach.

[davydden]

If not, then perhaps it would be worthwhile to turn this flag into a function like you suggested, but have it depend on the version of the compiler.

👍

[citibeth]

Thank you for the clarification. I agree, it belongs with the compiler.

On Mon, May 2, 2016 at 1:08 AM, Denis Davydov notifications@github.com
wrote:

We can't have our compiler classes depending on
specific packages (such as OpenMP).

I have a feeling that you misunderstand what openmp is. It is not a
package. It is an API which (from http://openmp.org/)

supports multi-platform parallel programming in C/C++ and Fortran.

with shared memory. State of affairs is that compilers have to
implement it if they want to allow users to utilize this. It is
absolutely opposite to mpi which is a library to do parallel
computation with distributed memory.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#876 (comment)

[citibeth]

There's been discussion of that, and it's on one of @tgamblin's long list
of things to maybe do in the future.

On Mon, May 2, 2016 at 9:51 AM, Adam J. Stewart notifications@github.com
wrote:

Personally, I would love to make compilers a build dependency. Then, that
compiler can provides('openmp', when='@Version:'), but I remember there
being significant push-back to that approach.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#876 (comment)

[davydden]

Ok, this worked for me on clang, i can do the same for other compilers

diff --git a/lib/spack/spack/compilers/clang.py b/lib/spack/spack/compilers/clang.py
index e406d86..44a7381 100644
--- a/lib/spack/spack/compilers/clang.py
+++ b/lib/spack/spack/compilers/clang.py
@@ -48,6 +48,13 @@ class Clang(Compiler):
                    'fc'  : 'f90' }

     @classmethod
+    def openmp_link_flags(self):
+        ver = self.default_version('clang')
+        if ver.endswith('-apple'):
+            raise RuntimeError('Clang from Apple does not support Openmp yet')
+        return '-fopenmp'
+
+    @classmethod
     def default_version(self, comp):
         """The '--version' option works for clang compilers.
            On most platforms, output looks like this::

or should it rather be @property ?

I agree that ideally Compiler should be derived from Package class so that we can also check C++11, C++14, and alike

provides('c11', when='@version:')

I think this would also facilitate mixing of c/c++ and fortran77/fortran compilers.