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

Add OpenMP support on clang / macOS #7435

Open
kirushyk opened this issue Jul 12, 2020 · 6 comments · May be fixed by #13079
Open

Add OpenMP support on clang / macOS #7435

kirushyk opened this issue Jul 12, 2020 · 6 comments · May be fixed by #13079
Labels
compilers dependencies enhancement OS:macos Issues specific to Apple Operating Systems like MacOS and iOS

Comments

@kirushyk
Copy link
Contributor

At this moment there is no way to detect OpenMP dependency on clang / macOS using Meson Build.
However OpenMP can be used with clang on macOS with brew install libomp and -Xpreprocess compiler option.

@lesteve
Copy link
Contributor

lesteve commented Apr 10, 2024

I added a small reproducer for this in https://github.com/lesteve/meson-openmp-osx.

cc @eli-schwartz that mentioned that this behaviour was unexpected in scikit-learn/scikit-learn#28710 (comment).

We are hitting this in scikit-learn: meson does not find OpenMP but the compilation with OpenMP actually works fine if the right compilation flags are set, see scikit-learn/scikit-learn#28710 for more details.

Looking around I have seen some work-arounds for this behaviour, in a few places for example dipy which has some brew-specific tweaks:
https://github.com/dipy/dipy/blob/95d75a8ad73fe76d7ebaa35b40ee8c734c9b267f/dipy/meson.build#L141-L157

Note that this does not only happen with OpenMP installed with brew but also when downloading OpenMP lib and include folder and setting compilation flags accordingly. This is what we are currently doing in the scikit-learn CI where we download OpenMP lib and include from conda-forge, see scikit-learn/scikit-learn#28710 (comment).

@dcbaker
Copy link
Member

dcbaker commented Apr 11, 2024

Is the -Xpreprocessor needed by GCC as well? or Vanilla (not Apple) Clang? Adding that argument to Meson's OpenMP wrapper should be easy enough. The more complicated question is dealing with the brew specific paths.

@lesteve
Copy link
Contributor

lesteve commented Apr 12, 2024

Not an expert, but I believe -Xpreprocessor is Apple Clang specific. Here are some notes about this by Henry Schreiner (a trustable source if you ask me) about this https://iscinumpy.gitlab.io/post/omp-on-high-sierra/.

Just to check, I did a quick test on https://github.com/lesteve/meson-openmp-osx with clang on Linux and Meson finds OpenMP (without setting any environment variable), compiles the test program and running the test program shows that OpenMP is indeed used.

Here is the meson setup output for completeness:

The Meson build system
Version: 1.4.0
Source dir: /home/lesteve/dev/meson-openmp-osx
Build dir: /home/lesteve/dev/meson-openmp-osx/build
Build type: native build
Project name: my-test
Project version: 1.0.0
C compiler for the host machine: clang (clang 17.0.6 "clang version 17.0.6")
C linker for the host machine: clang ld.bfd 2.42.0
Host machine cpu family: x86_64
Host machine cpu: x86_64
Run-time dependency OpenMP for c found: YES 5.1
Build targets in project: 1

Found ninja-1.11.1 at /home/lesteve/micromamba/envs/scikit-learn-dev/bin/ninja

@dcbaker dcbaker linked a pull request Apr 12, 2024 that will close this issue
@dcbaker
Copy link
Member

dcbaker commented Apr 12, 2024

Okay, that article is very helpful.

I've opened 13079 as a starting point, which adds the -Xpreprocessor (Unfortunately my Mac is very old and not at hand).

How exactly to handle the rest of it is more complicated. We could inject user supplied arguments into the dependencies, although that seems fragile and is going to be a big project to make sure we're doing that consistently.

dcbaker added a commit to dcbaker/meson that referenced this issue Apr 25, 2024
Which requires injecting some extra paths and the `-Xpreprocess` flag.

Fixes: mesonbuild#7435
dcbaker added a commit to dcbaker/meson that referenced this issue Apr 25, 2024
Which requires injecting some extra paths and the `-Xpreprocess` flag.

Fixes: mesonbuild#7435
dcbaker added a commit to dcbaker/meson that referenced this issue Apr 25, 2024
Which requires injecting some extra paths and the `-Xpreprocess` flag,
as well as extra search paths for libomp and the headers.

Fixes: mesonbuild#7435
@lpsinger
Copy link

Note that there is an exception: clang from MacPorts supports OpenMP, even without the -Xpreprocess option. See #13145.

@dcbaker
Copy link
Member

dcbaker commented Apr 26, 2024

This is particular to the Clang shipped by Apple as part of XCode, this abstraction should not get used for Clang from MacPorts or Homebrew or Nix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compilers dependencies enhancement OS:macos Issues specific to Apple Operating Systems like MacOS and iOS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants