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

Allow passing OpenMP root directory to -Dwith-openmp #3299

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

heplesser
Copy link
Contributor

This PR brings the handling of -Dwith-openmp in line with MPI and other libraries according to modern CMake practice, so that the root directory of the libomp installation can be passed as an argument. In existing code, and argument different from ON or OFF is actually handled incorrectly. Also update related variable names to include lowercase letters, as is modern CMake practice, see CMP0144.

Furthermore, in the cmake summary, results for threading and MPI are now reported next to each other early in the report.

@heplesser heplesser added T: Bug Wrong statements in the code or documentation S: High Should be handled next I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Aug 26, 2024
@heplesser heplesser requested review from lekshmideepu and gtrensch and removed request for lekshmideepu August 26, 2024 06:30
Copy link
Contributor

@gtrensch gtrensch left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks for this contribution.

@heplesser
Copy link
Contributor Author

@JoshuaBoettcher I have now added you as a reviewer and am looking forward to your comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: High Should be handled next T: Bug Wrong statements in the code or documentation
Projects
Status: Review
Development

Successfully merging this pull request may close these issues.

2 participants