[SYCL RTC] Use `cmake --install` for `resource.cpp` generation #20466

aelovikov-intel · 2025-10-24T19:07:12Z

Fixes #20275

That way we achieve all of:

Better "filtering" of what we need, as we don't operate at files/directories level anymore. Applies to both what we want to include and what we don't need (i.e., if provided by components we're not interested in)
Avoid race conditions with other targets creating temp files in the directories we take files from

Fixes intel#20275 That way we achieve all of: * Better "filtering" of what we need, as we don't operate at files/directories level anymore. Applies to both what we want to include and what we don't need (i.e., if provided by components we're not interested in) * Avoid race conditions with other targets creating temp files in the directories we take files from

aelovikov-intel · 2025-10-24T19:38:44Z

@sarnex, @jsj, added you as CMake experts.

sarnex

questions/nits

sarnex · 2025-10-24T20:01:05Z

sycl-jit/jit-compiler/CMakeLists.txt

+  )
+endforeach()
+
+add_custom_target(rtc-prepare-resources


It looks like add_custom_target is always considered out of date, so this will be run any time someone builds even if it's incremental. Is that a big deal? Is this expensive to run?

Is that a big deal?

Not sure

Is this expensive to run?

No (at least IMO), and the incremental builds will output this:

-- Up-to-date: <root>/build/tools/sycl-jit/jit-compiler/rtc-resources-install/lib/libsycl-fallback-imf-fp64.o

instead of

-- Installing: <root>/build/tools/sycl-jit/jit-compiler/rtc-resources-install/lib/libsycl-fallback-imf-fp64.o

I'm also thinking about rm -rf rtc-resources-install after resource.cpp is generated so that we won't have stale files in there. In that case we'd even want to run it all the time.

Ah I see, it will just call the target which itself will determine it is up to date, so basically nothing happens and it's better than before IMO

I'm also thinking about rm -rf rtc-resources-install after resource.cpp is generated so that we won't have stale files in there. In that case we'd even want to run it all the time.

Wouldn't the version of the file in that folder be regenerated if anyone makes a change to the file? Like someone modifies sycl.h and rebuilds and at that point it will rerun the resource.cpp generation in full because dependency sycl-headers or whatever changed right?

Updating sycl.hpp works with this version (AFAIK), I was thinking about deleting it. But, to be fair, we have that problem everywhere else, so probably no reason to address here.

sarnex · 2025-10-24T20:03:47Z

sycl-jit/jit-compiler/CMakeLists.txt

 add_custom_command(
  OUTPUT ${SYCL_JIT_RESOURCE_CPP}
-  COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/utils/generate.py --toolchain-dir ${CMAKE_BINARY_DIR} --output ${SYCL_JIT_RESOURCE_CPP} --prefix ${SYCL_JIT_VIRTUAL_TOOLCHAIN_ROOT}
+  COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/utils/generate.py --toolchain-dir ${CMAKE_CURRENT_BINARY_DIR}/rtc-resources-install --output ${SYCL_JIT_RESOURCE_CPP} --prefix ${SYCL_JIT_VIRTUAL_TOOLCHAIN_ROOT}


nit: could we make rtc-resources-install into a variable so its not copy pasted here and in the loop defining it?

sarnex · 2025-10-24T20:09:31Z

sycl-jit/jit-compiler/CMakeLists.txt

+set(ALL_RTC_PREPARE_RESOURCES_COMMANDS)
+foreach(component IN LISTS SYCL_JIT_RESOURCE_INSTALL_COMPONENTS)
+  list(APPEND ALL_RTC_PREPARE_RESOURCES_COMMANDS
+    COMMAND ${CMAKE_COMMAND} --install ${CMAKE_BINARY_DIR} --prefix ${CMAKE_CURRENT_BINARY_DIR}/rtc-resources-install --component "${component}"


So the idea here is we re-install all these components into a new rtc-resources-install subfolder and then run that script to embed them into something, and we need to do the separate install because using the actual installed files runs into race conditions because files are moved around?

Yes, except that before this PR we weren't even installing - just taking from the build directly.

Also, race is only one issue, better control of what's in there is beneficial on its own.

sarnex

lgtm with nit fixed

jsji

I like the idea of using cmake component install. Thanks!

jsji · 2025-10-24T21:46:45Z

sycl-jit/jit-compiler/CMakeLists.txt

-  sycl-headers    # include/sycl
-  clang           # lib/clang/N/include
-  ${CMAKE_CURRENT_SOURCE_DIR}/utils/generate.py)
+set(SYCL_JIT_RESOURCE_DEPS sycl-headers clang)


Why we don't need to depends on OpenCL-Headers?
Also do we need to depends on clang just for clang-resource-headers?

If we can depend on all the components to be installed, then we can just maintain one list instead two lists here?

Why we don't need to depends on OpenCL-Headers?

There is no such target, but I probably need to dig deeper still.

aelovikov-intel requested review from a team and cperkinsintel as code owners October 24, 2025 19:07

aelovikov-intel temporarily deployed to WindowsCILock October 24, 2025 19:07 — with GitHub Actions Inactive

aelovikov-intel temporarily deployed to WindowsCILock October 24, 2025 19:33 — with GitHub Actions Inactive

aelovikov-intel added the run-perf-tests Run performance tests in pre-commit (normally part of post-commit only) label Oct 24, 2025

aelovikov-intel requested review from jsji and sarnex October 24, 2025 19:38

sarnex reviewed Oct 24, 2025

View reviewed changes

aelovikov-intel added 2 commits October 24, 2025 13:26

SYCL_JIT_RESOURCE_INSTALL_DIR

73e9f6e

s/ALL_RTC_PREPARE_RESOURCES_COMMANDS/SYCL_JIT_PREPARE_RESOURCE_COMMANDS/

2b47019

sarnex approved these changes Oct 24, 2025

View reviewed changes

Drop COMMAND_EXPAND_LISTS

ce42833

aelovikov-intel had a problem deploying to WindowsCILock October 24, 2025 20:38 — with GitHub Actions Error

Drop now useless comments

648c87a

aelovikov-intel temporarily deployed to WindowsCILock October 24, 2025 20:48 — with GitHub Actions Inactive

aelovikov-intel temporarily deployed to WindowsCILock October 24, 2025 21:13 — with GitHub Actions Inactive

aelovikov-intel had a problem deploying to WindowsCILock October 24, 2025 21:13 — with GitHub Actions Failure

jsji approved these changes Oct 24, 2025

View reviewed changes

aelovikov-intel had a problem deploying to WindowsCILock October 24, 2025 23:12 — with GitHub Actions Failure

[SYCL RTC] Use cmake --install for resource.cpp generation #20466

Are you sure you want to change the base?

[SYCL RTC] Use cmake --install for resource.cpp generation #20466

Uh oh!

Conversation

aelovikov-intel commented Oct 24, 2025

Uh oh!

aelovikov-intel commented Oct 24, 2025

Uh oh!

sarnex left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sarnex Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sarnex left a comment

Choose a reason for hiding this comment

Uh oh!

jsji left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[SYCL RTC] Use `cmake --install` for `resource.cpp` generation #20466

[SYCL RTC] Use `cmake --install` for `resource.cpp` generation #20466

sarnex Oct 24, 2025 •

edited

Loading