[runtimes] Fix OpenMP dependencies #85977
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When building the OpenMP runtime with libomptarget support, the
runtimes configure step needs to have a dependency on various
tools, in particular opt, so that cmake configure checks yield
the correct results.
This did not work correctly, as the dependencies were only added
if the OPENMP_ENABLE_LIBOMPTARGET was set - but that variable is
only set by the openmp/CMakeLists.txt file, which isn't even
parsed during the initial cmake run (in fact, it is only parsed
when executing the runtimes configure step itself, but then it
is too late).
Fixed by just adding those dependencies always.
In addition, the list of dependencies collected in ${extra_deps},
including those required for OpenMP, was only actually used when
configuring runtimes for the default set of targets - when the
user specifies a non-default LLVM_RUNTIME_TARGETS, those extra
dependencies were ignored (with the exception of ${hdrgen_deps}).
Fixed by passing the full ${extra_deps} in this case as well.
Fixes: llvm#85933
chencha3
pushed a commit
to chencha3/llvm-project
that referenced
this pull request
Mar 23, 2024
When building the OpenMP runtime with libomptarget support, the runtimes
configure step needs to have a dependency on various tools, in
particular opt, so that cmake configure checks yield the correct
results.
This did not work correctly, as the dependencies were only added if the
OPENMP_ENABLE_LIBOMPTARGET was set - but that variable is only set by
the openmp/CMakeLists.txt file, which isn't even parsed during the
initial cmake run (in fact, it is only parsed when executing the
runtimes configure step itself, but then it is too late).
Fixed by just adding those dependencies always.
In addition, the list of dependencies collected in ${extra_deps},
including those required for OpenMP, was only actually used when
configuring runtimes for the default set of targets - when the user
specifies a non-default LLVM_RUNTIME_TARGETS, those extra dependencies
were ignored (with the exception of ${hdrgen_deps}).
Fixed by passing the full ${extra_deps} in this case as well.
Fixes: llvm#85933
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When building the OpenMP runtime with libomptarget support, the runtimes configure step needs to have a dependency on various tools, in particular opt, so that cmake configure checks yield the correct results.
This did not work correctly, as the dependencies were only added if the OPENMP_ENABLE_LIBOMPTARGET was set - but that variable is only set by the openmp/CMakeLists.txt file, which isn't even parsed during the initial cmake run (in fact, it is only parsed when executing the runtimes configure step itself, but then it is too late).
Fixed by just adding those dependencies always.
In addition, the list of dependencies collected in ${extra_deps}, including those required for OpenMP, was only actually used when configuring runtimes for the default set of targets - when the user specifies a non-default LLVM_RUNTIME_TARGETS, those extra dependencies were ignored (with the exception of ${hdrgen_deps}).
Fixed by passing the full ${extra_deps} in this case as well.
Fixes: #85933