Skip to content

[SYCL][NativeCPU][CMake] Simplify OCK CMake handling #19704

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

Open
wants to merge 1 commit into
base: sycl
Choose a base branch
from

Conversation

sarnex
Copy link
Contributor

@sarnex sarnex commented Aug 4, 2025

I'm trying to make this repo more easily packaged by Linux distros, and OCK came up in some feedback from NixOS (see linked issue).

Overall, the external dependency behavior should work as follows:

  1. Use the user specified directory if given
  2. If not, search the system for the dependency's install location and use that
  3. If not found, fetch it

OCK seems a bit special because installing OCK locally doesn't seem to install any CMake files nor does it install the files required to use it to build NativeCPU support. Seems we actually need the git tree, not the installed result of a built OCK. Strange but let's go with it.

You can already provide a precreated clone to use to build with FetchContent using FETCHCONTENT_SOURCE_DIR_<uppercaseName>, so there is no need for variable to set the directory and a custom repo and tag.

Remove variables that duplicate functionality that already exists in FetchContent, and add a simple search for the source directory on the system.

Issue: #19635

@sarnex sarnex temporarily deployed to WindowsCILock August 4, 2025 20:23 — with GitHub Actions Inactive
@sarnex sarnex temporarily deployed to WindowsCILock August 4, 2025 21:02 — with GitHub Actions Inactive
@sarnex sarnex temporarily deployed to WindowsCILock August 4, 2025 21:02 — with GitHub Actions Inactive
@sarnex sarnex marked this pull request as ready for review August 4, 2025 22:13
@sarnex sarnex requested a review from a team as a code owner August 4, 2025 22:13
)
FetchContent_GetProperties(oneapi-ck)
if(NOT oneapi-ck_POPULATED)
message(STATUS "Cloning oneAPI Construction Kit from ${OCK_GIT_INTERNAL_REPO}, tag ${OCK_GIT_INTERNAL_TAG}")
if(FETCHCONTENT_SOURCE_DIR_ONEAPI-CK)
message(STATUS "Using specified oneAPI Consutrction Kit repo location at ${FETCHCONTENT_SOURCE_DIR_ONEAPI-CK}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Consutrction -> Construction

@@ -31,8 +27,16 @@ if(NOT "native_cpu" IN_LIST SYCL_ENABLE_BACKENDS)
endif()

if(NATIVECPU_USE_OCK)
if(NATIVECPU_OCK_USE_FETCHCONTENT)
set(OCK_GIT_INTERNAL_REPO "https://github.com/uxlfoundation/oneapi-construction-kit.git")
set(OCK_SEARCH_LOC "oneapi-construction-kit/compiler_passes")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not overly convinced we should keep OCK_SEARCH_LOC since it was a convenience and isn't really tested. Feel free to keep though.

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