Repository navigation
Fix kernel launches on CPU devices - #61
Open
ChihweiLHBird wants to merge 3 commits into
Open
ChihweiLHBird wants to merge 3 commits into
ChihweiLHBird wants to merge 3 commits into
Conversation
ChihweiLHBird
marked this pull request as draft
November 1, 2026 01:19
ChihweiLHBird
force-pushed
the
cpu-pointer-args
branch
2 times, most recently
from
November 1, 2026 01:41
cdbc52a to
93339da
Compare
ChihweiLHBird
marked this pull request as ready for review
November 1, 2026 04:09
The launch functions in sycl-rs-sys took each kernel argument as plain bytes and always handed them to raw_kernel_arg, so the C++ shim could not tell a USM pointer from a value. A backend may take pointers through a different entry point than plain bytes: the OpenCL CPU device rejects USM pointers passed through raw_kernel_arg. Pass each argument as a KernelArg with a kind. Bytes still go through raw_kernel_arg, and pointers go through the typed handler::set_arg overload. sycl-rs passes every argument as Bytes for now, so behaviour is unchanged. Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>
KernelArgument::as_raw_arg and KernelArgumentList::as_raw_arg_list returned plain byte slices. Return a RawKernelArg instead, so that a later change can mark USM pointers. Every argument is still passed as bytes, so behaviour is unchanged. RawKernelArg is non-exhaustive so that more kinds can be added without a breaking change. Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>
UsmBox kernel arguments were passed as plain bytes. On the OpenCL CPU device, kernel launches with them failed with UR_RESULT_ERROR_INVALID_MEM_OBJECT, so no kernel could run on a CPU. Add RawKernelArg::Pointer, return it for UsmBox arguments, and require KernelArgument and KernelArgumentList implementations to mark USM pointers this way. The launch bridge then passes them through handler::set_arg. Since the pointer bytes now matter, take their size from the NonNull<T> field instead of an unrelated *const u8. Add a regression test, which fails without this change on the OpenCL CPU device. It prefers the CPU device, because the default device is a GPU when there is one, and GPUs accepted pointers passed as plain bytes. Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>
ChihweiLHBird
force-pushed
the
cpu-pointer-args
branch
from
November 3, 2026 05:48
93339da to
c8c5b58
Compare
Contributor
|
Hi @ChihweiLHBird, it's great to see you contributing to oneAPI-rs. Are you interested in using this crate in your projects? |
bratpiorka
requested review from
bratpiorka and
szymon-zadworny
and
a balanced review from Copilot
November 4, 2026 15:30
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
CPU use of raw_kernel_arg, documented as GPU-only, requires regression testing on the affected oneAPI 2026.1 Intel OpenCL CPU runtime and confirmation of the existing GPU path beyond source review.
No open issues 🎉
What changed in this PR
Fixes #62 by distinguishing USM pointers from plain-value kernel arguments throughout the Rust-to-SYCL launch bridge, allowing launches on OpenCL CPU devices.
Changes:
- Introduces
RawKernelArgand carries argument kinds through traits, derives, and launch interfaces. - Passes USM pointers through typed
handler::set_arg, while retainingraw_kernel_argfor plain values. - Adds a CPU-preferred regression test mixing scalar and pointer arguments.
| File | Description |
|---|---|
sycl/sycl-rs/tests/launch.rs |
Adds a CPU-preferred SAXPY launch regression test. |
sycl/sycl-rs/src/usmbox.rs |
Marks owned and borrowed USM arguments as pointers. |
sycl/sycl-rs/src/range.rs |
Converts typed arguments for all launch dimensions. |
sycl/sycl-rs/src/prelude.rs |
Exports RawKernelArg through the prelude. |
sycl/sycl-rs/src/kernel.rs |
Defines argument kinds and updates argument traits. |
sycl/sycl-rs-sys/src/types-sys.rs |
Adds argument-kind and argument-record bridge types. |
sycl/sycl-rs-sys/src/queue.cpp |
Dispatches pointer and value arguments separately. |
sycl/sycl-rs-sys/src/queue-sys.rs |
Updates launch bindings and safety contracts. |
sycl/sycl-rs-sys/include/queue.hpp |
Updates C++ launch declarations for typed arguments. |
sycl/sycl-rs-derive/src/lib.rs |
Updates generated argument lists to preserve argument kinds. |
CHANGELOG.md |
Documents pointer-aware kernel argument handling. |
Review effort: Balanced
Author
|
Hi @bratpiorka, yes I plan to use this crate in a prototype/learning project. No rush, and let me know if you need anything from my side. Thank you so much! |
This branch has not been deployed
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
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.
Fixes #62
Change
USM pointers are marked as pointers and passed with
handler::set_arg. Plain values still useraw_kernel_arg. The spec's newerpointer_argform isn't in oneAPI 2026.1.RawKernelArgfromKernelArgument. No behaviour change.UsmBoxarguments as pointers, and add a regression test that prefers the CPU device.