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

OpenMP runtimes #831

Closed
isuruf opened this issue Aug 5, 2019 · 23 comments
Closed

OpenMP runtimes #831

isuruf opened this issue Aug 5, 2019 · 23 comments

Comments

@isuruf
Copy link
Member

isuruf commented Aug 5, 2019

There are 3 variants,

  1. libgcc-ng - libgomp.so
  2. intel-openmp - libiomp5.so
  3. llvm-openmp - libomp.so

2 and 3 are basically the same. They share the same codebase. intel-openmp-2019.4-243 and llvm-openmp-8.0.0 have the same symbols except for kmp_compose_init which is missing in llvm-openmp. (No google hits on that symbol)

One advantage of LLVM and Intel's openmp is that it is fork safe, while GNU's is not.

LLVM's openmp have two sets of symbols. GOMP ones and Intel ones. clang generated code use the Intel symbols. The GOMP symbols are present in LLVM's openmp which means we can swap the two implementations.

I checked libgcc-ng 7.3.0 and llvm-openmp 8.0.0 and the symbols in https://gist.github.com/isuruf/36851d30ef57153807a8bdd11812ee90 are missing. Most of these relate to OpenACC and target off-loading in OpenMP 4.5. (There are some symbols missing that should be fixed in LLVM side. For eg: https://reviews.llvm.org/D65714)

My question is are we okay with going the BLAS way and swapping these implementations under the hood? BLAS is working really well and I thought we could use the same strategy for openmp.

cc @mingwandroid, @conda-forge/core

@scopatz
Copy link
Member

scopatz commented Aug 6, 2019

I am OK with swapping implementations.

@mingwandroid
Copy link

mingwandroid commented Aug 8, 2019

Me too, but I'd love it if we can coordinate timing of binaries [1] and recipes, maybe around 8.0.1 and then do a coordinated release of some core packages.

@isuruf what are your thoughts on libc++ for Linux? I feel we need to move away from GCC and everything that's a part of that toolchain eventually since they appear to have little intention of fixing the libgomp race condition that's existed for years.

[1] I am OK with using CF compiler binaries personally but I can't promise that for Anaconda Distro yet

@isuruf
Copy link
Member Author

isuruf commented Aug 8, 2019

@isuruf what are your thoughts on libc++ for Linux? I feel we need to move away from GCC and everything that's a part of that toolchain eventually since they appear to have little intention of fixing the libgomp race condition that's existed for years.

A huge NO from me. libstdc++ is the standard and I'd like to keep using conda packages and system packages locally. libgomp race condition can be fixed by swapping the implementation I mentioned above.

@mingwandroid
Copy link

mingwandroid commented Aug 8, 2019

Fair enough. I don't think MandrivaLinux have switched to libc++ yet and I've been using that as a yardstick for when such an idea isn't just plain daft. It hasn't happened yet, only some BSDs and macOS use libc++ so far AFAIK.

edit: For those interested in this: https://llvm.org/devmtg/2019-04/slides/TechTalk-Rosenkranzer-Switching_a_Linux_distro.pdf

As an aside I once sat beside Bero at the first Qt contributors summit where we discussed finishing porting Qt to Android. He was very nice but our accents clashed badly!

@isuruf
Copy link
Member Author

isuruf commented Aug 12, 2019

libgomp.dylib and libomp.dylib have different compatibility version numbers on OSX, so we can't switch libraries already built.

My proposal is to switch for gfortran-7 on OSX in conda-forge/gfortran_impl_osx-64-feedstock#11

cc @beckermr

@beckermr
Copy link
Member

I have rebuilt a few bits of the stack with the new compiler already to test things. We need to make sure the migration bot proposes a rebuild for them anyways after the PR above and it actually gets done. The packages are openblas, lapack and scipy.

@isuruf
Copy link
Member Author

isuruf commented Aug 12, 2019

@beckermr, thanks for the heads up. None of them are built with gfortran openmp. So we are good.

@beckermr
Copy link
Member

Ahh cool. I have one downstream package (https://github.com/conda-forge/camb-feedstock) that may use gfortran openmp. I will bump it by hand.

@jakirkham
Copy link
Member

One advantage of LLVM and Intel's openmp is that it is fork safe, while GNU's is not.

Has this been confirmed somewhere?

@isuruf
Copy link
Member Author

isuruf commented Sep 9, 2019

@isuruf
Copy link
Member Author

isuruf commented Dec 31, 2019

For anyone wanting to take this up, here are the steps needed,

  1. Split libgcc-ng output in https://github.com/conda-forge/ctng-compilers-feedstock/blob/libgcc-and-libstdcxx-9/recipe/meta.yaml to libgcc-ng, libgomp and openmp_impl=4.5=0_gomp.
    libgcc-ng should contain libgomp.so, libgomp should contain libgomp.so.1.0.0 and openmp_impl should contain libgomp.so.1.

  2. Add an output openmp_impl=4.5=0_llvm to https://github.com/conda-forge/openmp-feedstock which would have a symlink libgomp.so.1 -> libomp.so

@beckermr
Copy link
Member

beckermr commented Jan 1, 2020

Recording this convo for posterity/documentation:

Isuru Fernando @isuruf 05:48
@davidbrochart, njsmith had a patch for to fix libgomp at https://gcc.gnu.org/ml/gcc-patches/2014-02/msg00813.html , but that's probably outdated now.
Looks like the numpy issue is fixed, and is unrelated, but the fork()-safety issue is still there in libgomp

David Brochart @davidbrochart 05:55
ok so another option is to patch GNU's openmp with @njsmith's (updated) fix. I could try that first. Thanks @isuruf !

Ray Donnelly @mingwandroid 06:44
@davidbrochart that would be an excellent thing to try to fix!

David Brochart @davidbrochart 06:44
 I will try!

@beckermr
Copy link
Member

beckermr commented Jan 1, 2020

One PR is here: conda-forge/openmp-feedstock#29

@beckermr
Copy link
Member

beckermr commented Jan 1, 2020

the other PR is here: conda-forge/ctng-compilers-feedstock#7

@isuruf
Copy link
Member Author

isuruf commented Jan 10, 2020

It's possible to switch between LLVM and GNU implementations on linux64 and ppc64le (No support for aarch64).

Updated docs on #953

@beckermr
Copy link
Member

I think we can close this issue now.

@isuruf
Copy link
Member Author

isuruf commented Jan 10, 2020

We'll need to get rid of intel-openmp on MKL as well

@beckermr
Copy link
Member

Do we have an MKL feedstock?

@isuruf
Copy link
Member Author

isuruf commented Jan 10, 2020

No, but we can steal https://github.com/AnacondaRecipes/intel_repack-feedstock
@scopatz said that NumFOCUS lawyer gave us a green light to redistribute MKL.

@beckermr
Copy link
Member

Small detail here @isuruf, but Anaconda Inc. has not applied a license to the MKL/intel_repack recipe itself. We'll need @msarahan, @jjhelmus, @soapy1, and @mingwandroid to apply a license to the recipe before we can ship it I think.

@isuruf
Copy link
Member Author

isuruf commented Jan 21, 2020

@beckermr, they are covered by https://github.com/AnacondaRecipes/aggregate/blob/master/LICENSE.txt

@beckermr
Copy link
Member

MKL is done now. Shall we finally close this one? :)

@beckermr
Copy link
Member

beckermr commented Dec 1, 2020

Closing!

@beckermr beckermr closed this as completed Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

6 participants