Fix OMPT build problem when OpenMP is enabled #177
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
I'm trying to build APEX with GCC (via spack) and when I enable OpenMP support I get a building error due to a missing include.
Quick fix
I had a quick look at it, and I have a quick fix for my use-case.
Since
OMP_INCLUDE_DIRS
is set byFindOMPT
in any case OMPT is found,apex/cmake/Modules/FindOMPT.cmake
Lines 118 to 123 in 0cc0ac9
I propose to add unconditionally the include directories to the target
apex_ompt
as per this PR.Notes
include_directories
directive can be removedapex/src/apex/CMakeLists_standalone.cmake
Line 118 in 0cc0ac9
I'm not sure if this change, if correct, should be applied somehow also to
src/apex/CMakeLists_hpx.cmake
.I'm going to open a PR also in spack about this. Depending on the outcome of this PR, I would add a conflict or a patch to the spack package.
Side note:
FindOMPT
(and probably it is not alone) installs the OMPT as external project in installation path instead of the build folder. If a user setCMAKE_INSTALL_PREFIX
(like spack does, but it might be also set to the classic/usr/local/apex
), the installation folder will get also OMPT sources and libraries, which I'm not sure it is needed. But this might be a topic for a different issue/PR.