Skip to content

Detect OpenMP on macOS with Apple's Clang#13079

Closed
dcbaker wants to merge 7 commits intomesonbuild:masterfrom
dcbaker:submit/openmp-detection-macos
Closed

Detect OpenMP on macOS with Apple's Clang#13079
dcbaker wants to merge 7 commits intomesonbuild:masterfrom
dcbaker:submit/openmp-detection-macos

Conversation

@dcbaker
Copy link
Copy Markdown
Member

@dcbaker dcbaker commented Apr 12, 2024

This has been opened as a draft because it is probably not sufficient yet, but it's a start.

Fixes #7435

@dcbaker dcbaker added OS:macos Issues specific to Apple Operating Systems like MacOS and iOS dependencies labels Apr 12, 2024
@dcbaker dcbaker requested a review from jpakkane as a code owner April 12, 2024 17:59
@dcbaker dcbaker marked this pull request as draft April 12, 2024 17:59
@dcbaker dcbaker force-pushed the submit/openmp-detection-macos branch from 5495b0f to fd7d240 Compare April 12, 2024 18:00


class AppleClangCCompiler(ClangCCompiler):
class AppleClangCCompiler(AppleCompilerMixin, ClangCCompiler):

Check warning

Code scanning / CodeQL

Conflicting attributes in base classes

Base classes have conflicting values for attribute 'openmp_flags': [Function openmp_flags](1) and [Function openmp_flags](2).


class AppleClangCPPCompiler(ClangCPPCompiler):
class AppleClangCPPCompiler(AppleCompilerMixin, ClangCPPCompiler):

Check warning

Code scanning / CodeQL

Conflicting attributes in base classes

Base classes have conflicting values for attribute 'openmp_flags': [Function openmp_flags](1) and [Function openmp_flags](2).
@dcbaker dcbaker force-pushed the submit/openmp-detection-macos branch 3 times, most recently from a28f395 to 8ebabc6 Compare April 25, 2024 22:17
dcbaker added 6 commits April 25, 2024 15:45
This will be needed by the Apple compiler
In case the OpenMP definition adds things like preprocessor directives
or include paths, like when building on MacOS with OpenMP from Homebrew.
It was probably done this way originally since we didn't have the
`fatal` keyword argument to avoid triggering the fatal-meson-warnings.

While we're here, replace the use of a `if bool` with an `else` on the
for loop.
Which requires injecting some extra paths and the `-Xpreprocess` flag,
as well as extra search paths for libomp and the headers.

Fixes: mesonbuild#7435
@dcbaker dcbaker force-pushed the submit/openmp-detection-macos branch from 8ebabc6 to ce6ea14 Compare April 25, 2024 22:49
Instead of making the variable optional, just return if we hit the error
case, since that's all that's going to happen anyway
@dcbaker dcbaker marked this pull request as ready for review April 25, 2024 23:05
@dcbaker
Copy link
Copy Markdown
Member Author

dcbaker commented Apr 25, 2024

@lesteve, this seems to be working in our CI, does this this work for you?

@tristan957
Copy link
Copy Markdown
Member

Looks like this needs a rebase. Than I can review it

@lesteve
Copy link
Copy Markdown
Contributor

lesteve commented May 21, 2024

@lesteve, this seems to be working in our CI, does this this work for you?

I can confirm that this PR fixes my original issue. In particular with my minimal reproducer repo https://github.com/lesteve/meson-openmp-osx, OpenMP is detected without having to set any environment variable. Also I checked the compiled binary does indeed use multiple threads.

Sorry it took me a while to get back to this ...

@lesteve
Copy link
Copy Markdown
Contributor

lesteve commented Jun 21, 2024

Looks like this branch now has conflicts, but quickly looking at this, they seem straightforward to fix.

Let me know if there is something I can do to help move this forward!

@tristan957
Copy link
Copy Markdown
Member

tristan957 commented Jun 24, 2024

Could be good to get this to 1.5. I know @dcbaker is a busy guy, and would never not want help. Could you take his branch, rebase it, and add that it supersedes this PR? Might be the quickest way to resolution.

@lesteve
Copy link
Copy Markdown
Contributor

lesteve commented Jun 24, 2024

Could you take his branch, rebase it, and add that it supersedes this PR? Might be the quickest way to resolution.

Thanks @tristan957, I have opened #13350 that does this.

@dcbaker
Copy link
Copy Markdown
Member Author

dcbaker commented Jun 24, 2024

Superseded by #13350

@dcbaker dcbaker closed this Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies OS:macos Issues specific to Apple Operating Systems like MacOS and iOS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add OpenMP support on clang / macOS

4 participants