Skip to content

[offload][runtimes] Forward user-provided system configuration. #96303

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

Merged
merged 2 commits into from
Jun 21, 2024

Conversation

Meinersbur
Copy link
Member

@Meinersbur Meinersbur commented Jun 21, 2024

In order for LLVM_ENABLE_RUNTIMES projects to find their requirements, they need access to user-provided configuration options such as CMAKE_PREFIX_PATH. Forward a selection of configuration options sich that runtimes uses the same system introspection as LLVM and LLVM_ENABLE_PROJECTS do.

The concrete symptom this is solving is that the path to CUDA is provided using cmake -DCUDA_TOOLKIT_ROOT_DIR=/opt/cuda or CUDA_PATH, but is ignored by offload. Handling for this case already existed for libc, but only when it was enabled and only CUDAToolkit_ROOT (The former is for find_package(CUDA), the latter for find_package(CUDAToolkit), CUDA_PATH is used by find_package(CUDAToolkit) and enable_language(CUDA)).

CUDA_TOOLKIT_ROOT_DIR is actually deprecated but still used and required by offload/test/lit.site.cfg.in.

@Meinersbur Meinersbur marked this pull request as ready for review June 21, 2024 13:18
@jhuber6
Copy link
Contributor

jhuber6 commented Jun 21, 2024

CUDA_TOOLKIT_ROOT_DIR is actually deprecated but still used and required by offload/list/lit.site.cfg.in.

We should probably fix that.

@DavidSpickett
Copy link
Collaborator

Seems like a generalisation of the code already added for libc, so no objections from me here.

@jhuber6 should be the approver, as the author of the original code.

@Meinersbur
Copy link
Member Author

Meinersbur commented Jun 21, 2024

We should probably fix that.

3de162f sets it to CUDAToolkit_BIN_DIR, but the meaning it previously had (the path passed to clang --cuda-path) would be CUDAToolkit_LIBRARY_ROOT.

@jhuber6
Copy link
Contributor

jhuber6 commented Jun 21, 2024

3de162f sets it to CUDAToolkit_BIN_DIR, but the meaning it previously had (the path passed to clang --cuda-path) would be CUDAToolkit_LIBRARY_ROOT.

No, it takes the directory of opt/cuda/bin which is opt/cuda for example.

@Meinersbur Meinersbur merged commit af478c8 into llvm:main Jun 21, 2024
4 of 6 checks passed
@Meinersbur
Copy link
Member Author

3de162f sets it to CUDAToolkit_BIN_DIR, but the meaning it previously had (the path passed to clang --cuda-path) would be CUDAToolkit_LIBRARY_ROOT.

No, it takes the directory of opt/cuda/bin which is opt/cuda for example.

OK, it does get_filename_component of CUDAToolkit_BIN_DIR, but would be unnecessary since CUDAToolkit_LIBRARY_ROOT already contains that path. CUDA_LIBDIR is also the same as CUDAToolkit_LIBRARY_DIR. See https://cmake.org/cmake/help/latest/module/FindCUDAToolkit.html#result-variables

@jhuber6
Copy link
Contributor

jhuber6 commented Jun 21, 2024

3de162f sets it to CUDAToolkit_BIN_DIR, but the meaning it previously had (the path passed to clang --cuda-path) would be CUDAToolkit_LIBRARY_ROOT.

No, it takes the directory of opt/cuda/bin which is opt/cuda for example.

OK, it does get_filename_component of CUDAToolkit_BIN_DIR, but would be unnecessary since CUDAToolkit_LIBRARY_ROOT already contains that path. CUDA_LIBDIR is also the same as CUDAToolkit_LIBRARY_DIR. See https://cmake.org/cmake/help/latest/module/FindCUDAToolkit.html#result-variables

Oh yeah, now I remember. I used the old one because that came in CMake 3.18 so it was the only solution before we moved to 3.20. We can definitely use the "official" one now, but probably doesn't make much of a difference.

AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
…96303)

In order for LLVM_ENABLE_RUNTIMES projects to find their requirements,
they need access to user-provided configuration options such as
`CMAKE_PREFIX_PATH`. Forward a selection of configuration options such
that runtimes uses the same system introspection as LLVM and
LLVM_ENABLE_PROJECTS do.

The concrete symptom this is solving is that the path to CUDA is
provided using `cmake -DCUDA_TOOLKIT_ROOT_DIR=/opt/cuda` or `CUDA_PATH`,
but is ignored by offload. Handling for this case already existed for
libc, but only when it was enabled and only `CUDAToolkit_ROOT` (The
former is for `find_package(CUDA)`, the latter for
`find_package(CUDAToolkit)`, `CUDA_PATH` is used by
`find_package(CUDAToolkit)` and `enable_language(CUDA)`).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants