Skip to content

[UR][CMake] Fix detection of preinstalled OpenCL#21531

Merged
sarnex merged 2 commits intointel:syclfrom
sarnex:oclfix2
Mar 20, 2026
Merged

[UR][CMake] Fix detection of preinstalled OpenCL#21531
sarnex merged 2 commits intointel:syclfrom
sarnex:oclfix2

Conversation

@sarnex
Copy link
Copy Markdown
Contributor

@sarnex sarnex commented Mar 16, 2026

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: #21513

@sarnex sarnex force-pushed the oclfix2 branch 5 times, most recently from 52d9143 to 0561ef0 Compare March 19, 2026 20:19
Signed-off-by: Nick Sarnie <nick.sarnie@intel.com>
@sarnex sarnex marked this pull request as ready for review March 19, 2026 21:18
@sarnex sarnex requested a review from a team as a code owner March 19, 2026 21:18
Copy link
Copy Markdown
Contributor

@lukaszstolarczuk lukaszstolarczuk left a comment

Choose a reason for hiding this comment

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

besides that, LGTM

Comment thread unified-runtime/cmake/FetchOpenCL.cmake Outdated
Comment thread unified-runtime/cmake/FetchOpenCL.cmake Outdated
Comment thread unified-runtime/cmake/FetchOpenCL.cmake Outdated
Signed-off-by: Nick Sarnie <nick.sarnie@intel.com>
@lukaszstolarczuk
Copy link
Copy Markdown
Contributor

heh, funny thing, we test opencl adapter on ubuntu22.04 image and it has cmake 3.22.1. Even though we install opencl in the workflow it still fetches it in CMake: https://github.com/intel/llvm/actions/runs/23354242003/job/67946717826?pr=21531#step:9:74

perhaps a next step should be to bump docker images...? I'm not sure if we have any requirement on min. version of Ubuntu to use...? perhaps only one of two opencl jobs...?

@sarnex
Copy link
Copy Markdown
Contributor Author

sarnex commented Mar 20, 2026

That's expected since we only support system installed with CMake 3.25 or later, so I can just bump the CMake version used in the Docker image, will do that in a separate PR now.

@sarnex
Copy link
Copy Markdown
Contributor Author

sarnex commented Mar 20, 2026

Actually it seems the OpenCL install there is for the CPU runtime, not the headers/ICD loader, but I can bump the version anyway

@lukaszstolarczuk
Copy link
Copy Markdown
Contributor

Actually it seems the OpenCL install there is for the CPU runtime, not the headers/ICD loader, but I can bump the version anyway

ohh, I see, that makes sense

Copy link
Copy Markdown
Contributor

@lukaszstolarczuk lukaszstolarczuk left a comment

Choose a reason for hiding this comment

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

LGTM

@sarnex sarnex merged commit 5c7bdc8 into intel:sycl Mar 20, 2026
90 of 94 checks passed
@sarnex
Copy link
Copy Markdown
Contributor Author

sarnex commented Mar 20, 2026

thanks for the review!

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.

Default build does not use system installed OpenCL

2 participants