Skip to content

[SYCL] Embed fsycl-id-queries-range in device image as property.#21979

Open
uditagarwal97 wants to merge 8 commits into
syclfrom
private/udit/kernel_prop_id_range
Open

[SYCL] Embed fsycl-id-queries-range in device image as property.#21979
uditagarwal97 wants to merge 8 commits into
syclfrom
private/udit/kernel_prop_id_range

Conversation

@uditagarwal97
Copy link
Copy Markdown
Contributor

@uditagarwal97 uditagarwal97 commented May 10, 2026

Problem
Consider the following case:
(1) Compile a.cpp with a kernel foo (say, in a functor), with -fsycl-id-queries-range=uint
(2) Compile b.cpp that submits kernel foo, without -fsycl-id-queries-range option (by default that means ID queries/ranges are limited to INT_MAX)
(3) Link the .o files from (1) and (2).

Currently, we emit the check whether id/ranges are in bounds, at compile-time, so when compiling b.cpp, we'd emit check to make sure SYCL queries/id are less than INT_MAX, even though the kernel we are submitting (from a.cpp) can support range upto UINT_MAX.

Solution
Store -fsycl-id-queries-range as a device image property and do the range checks at runtime.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR shifts -fsycl-id-queries-range enforcement from a host-side header check to an enqueue-time runtime check by embedding the selected mode into the device image as a binary property and reading it during kernel submission.

Changes:

  • Add an idQueriesRange misc property to SYCL device images (emitted by sycl-post-link, driven by a new post-link option forwarded from clang).
  • Validate launch ranges at enqueue time in the scheduler using the embedded property; remove the old header-based checkValueRange mechanism and associated include-deps expectations.
  • Remove the dedicated sycl/unittests/range/* unit tests and their CMake wiring.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
sycl/source/detail/scheduler/commands.cpp Fetches idQueriesRange property from the device image and performs enqueue-time range validation.
sycl/source/detail/ndrange_desc.hpp Adds getNumGlobalWorkGroups() helper used by the new validation logic.
sycl/source/detail/device_binary_image.cpp Adjusts dynamic image merge behavior for misc properties (now also mentions idQueriesRange).
llvm/tools/sycl-post-link/sycl-post-link.cpp Introduces -id-queries-range= option and emits the property via GlobalBinImageProps.
llvm/lib/SYCLPostLink/ComputeModuleRuntimeInfo.cpp Emits idQueriesRange into the device image misc property set.
llvm/include/llvm/SYCLPostLink/ComputeModuleRuntimeInfo.h Extends GlobalBinImageProps with IdQueriesRange.
clang/lib/Driver/ToolChains/Clang.cpp Forwards -fsycl-id-queries-range= to sycl-post-link as -id-queries-range=.
sycl/include/sycl/queue.hpp Removes host-side checkValueRange usage and the dependency on the deleted header.
sycl/include/sycl/handler.hpp Removes host-side checkValueRange usage and the dependency on the deleted header.
sycl/include/sycl/detail/id_queries_fit_in_int.hpp Deletes the old header implementing host-side range validation.
sycl/test/include_deps/sycl_khr_includes_*.hpp.cpp and sycl/test/include_deps/sycl_detail_core.hpp.cpp Updates include dependency expectations after removing id_queries_fit_in_int.hpp.
sycl/unittests/CMakeLists.txt Removes add_subdirectory(range) from the SYCL unit test suite.
sycl/unittests/range/** Deletes the -fsycl-id-queries-range range validation unit tests and their CMake glue.

Comment thread sycl/source/detail/scheduler/commands.cpp Outdated
Comment thread sycl/source/detail/scheduler/commands.cpp Outdated
Comment thread sycl/source/detail/scheduler/commands.cpp Outdated
Comment thread sycl/source/detail/ndrange_desc.hpp Outdated
Comment thread sycl/source/detail/ndrange_desc.hpp
Comment thread sycl/source/detail/device_binary_image.cpp Outdated
Comment thread llvm/include/llvm/SYCLPostLink/ComputeModuleRuntimeInfo.h Outdated
Comment thread sycl/unittests/CMakeLists.txt
@uditagarwal97 uditagarwal97 marked this pull request as ready for review May 13, 2026 21:20
@uditagarwal97 uditagarwal97 requested review from a team and cperkinsintel as code owners May 13, 2026 21:20
Copy link
Copy Markdown
Contributor

@kswiecicki kswiecicki left a comment

Choose a reason for hiding this comment

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

UR code LGTM

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