Skip to content

[UR][L0] Set pointer argument properly in multi-device scenario #19225

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 2 commits into
base: sycl
Choose a base branch
from

Conversation

againull
Copy link
Contributor

USM pointer to the allocation on a device has to be set only for the L0 kernel handle associated with that device or it's sub-devices. Before this change argument was set for all kernel handles leading to an error if device of the kernel handle and allocation device are different.

@againull againull requested review from a team as code owners June 30, 2025 23:01
@pbalcer
Copy link
Contributor

pbalcer commented Jul 1, 2025

USM pointer to the allocation on a device has to be set only for the L0 kernel handle associated with that device or it's sub-devices. Before this change argument was set for all kernel handles leading to an error if device of the kernel handle and allocation device are different.

This doesn't sound right to me. I think the argument should be set for all devices that can access the memory (which is not just parent and subdevices).

Also, isn't setting inaccessible arguments to kernels a user error from UR perspective? SYCL spec says this:

When a group of devices can be grouped together on the same context, they have some visibility of each other’s memory objects. The SYCL runtime can assume that memory is visible across all devices in the same context. Not all devices exposed from the same platform can be grouped together in the same context.

https://registry.khronos.org/SYCL/specs/sycl-2020/html/sycl-2020.html#sec:platformmodel

To me this sounds like we should simply disallow creating a context in this scenario.

@againull
Copy link
Contributor Author

againull commented Jul 1, 2025

@pbalcer

This doesn't sound right to me. I think the argument should be set for all devices that can access the memory (which is not just parent and subdevices).

According to description of zeMemAllocDevice in L0 spec:
https://oneapi-src.github.io/level-zero-spec/level-zero/latest/core/api.html#zememallocdevice

"The application must only use the memory allocation for the context and device, or its sub-devices, which was provided during allocation."

@igchor
Copy link
Member

igchor commented Jul 1, 2025

@pbalcer

This doesn't sound right to me. I think the argument should be set for all devices that can access the memory (which is not just parent and subdevices).

According to description of zeMemAllocDevice in L0 spec: https://oneapi-src.github.io/level-zero-spec/level-zero/latest/core/api.html#zememallocdevice

"The application must only use the memory allocation for the context and device, or its sub-devices, which was provided during allocation."

But wouldn't that mean that device-to-device (p2p) access is impossible? We actually rely on accessing memory from different device in v2 buffer implementation here: https://github.com/intel/llvm/blob/sycl/unified-runtime/source/adapters/level_zero/v2/memory.cpp#L286 (not sure if this is properly tested on SYCL level 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.

4 participants