[UR][CMake] Fix detection of preinstalled OpenCL (#21531)#21967
Open
sarnex wants to merge 1 commit intointel:sycl-rel-7_0from
Open
[UR][CMake] Fix detection of preinstalled OpenCL (#21531)#21967sarnex wants to merge 1 commit intointel:sycl-rel-7_0from
sarnex wants to merge 1 commit intointel:sycl-rel-7_0from
Conversation
There were a couple problems with the OpenCL detection. 1) System install detection didn't work because I had to add the `NO_CMAKE_PACKAGE_REGISTRY` argument to `find_package` to prevent it finding the `FetchContent` checkout on subsequent calls. This CMake module is called at least twice, one for `opencl-aot` and once for the UR OpenCL adapter. 2) If we remove `NO_CMAKE_PACKAGE_REGISTRY` to fix system install detection, then we hit a problem where if there is a system install, but it fails the `check_cxx_source_compiles` check because it doesn't support `cl_khr_spirv_queries`, we had no way to not use the system install it found, so we can't use `find_package` at all. Use `find_path` and `find_library`, whcih is what `find_package(OpenCL)` does internally anyway. Those two functions have nice validator functions we can use that do exactly what we need, to determine if the found OpenCL install is suitable, and if not, don't use it. However, this function is only available with CMake 3.25 (released in 2022) or later, and we currently require `3.20`, but I think saying we don't support system installs with older CMake is reasonable, it still fetches and builds OpenCL from upstream fine. Also do some minor cleanup like making sure there is always a target named `OpenCL` that callers can rely on. Tested the following 3 cases manually: a) No system OpenCL install b) System OpenCL install that's too old c) Working system OpenCL install and all work as expected. Closes: intel#21513 --------- Signed-off-by: Nick Sarnie <nick.sarnie@intel.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Cherry pick of 5c7bdc8
There were a couple problems with the OpenCL detection.
System install detection didn't work because I had to add the
NO_CMAKE_PACKAGE_REGISTRYargument tofind_packageto prevent it finding theFetchContentcheckout on subsequent calls. This CMake module is called at least twice, one foropencl-aotand once for the UR OpenCL adapter.If we remove
NO_CMAKE_PACKAGE_REGISTRYto fix system install detection, then we hit a problem where if there is a system install, but it fails thecheck_cxx_source_compilescheck because it doesn't supportcl_khr_spirv_queries, we had no way to not use the system install it found, so we can't usefind_packageat all. Usefind_pathandfind_library, whcih is whatfind_package(OpenCL)does internally anyway. Those two functions have nice validator functions we can use that do exactly what we need, to determine if the found OpenCL install is suitable, and if not, don't use it. However, this function is only available with CMake 3.25 (released in 2022) or later, and we currently require3.20, but I think saying we don't support system installs with older CMake is reasonable, it still fetches and builds OpenCL from upstream fine.Also do some minor cleanup like making sure there is always a target named
OpenCLthat callers can rely on.Tested the following 3 cases manually:
a) No system OpenCL install
b) System OpenCL install that's too old
c) Working system OpenCL install
and all work as expected.
Closes: #21513