Skip to content

[SYCL][Graph][UR] Propagate graph update list to UR #17019

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

Merged
merged 4 commits into from
Mar 4, 2025

Conversation

EwanC
Copy link
Contributor

@EwanC EwanC commented Feb 14, 2025

Update the urCommandBufferUpdateKernelLaunchExp API for updating commands in a command-buffer to take a list of commands.

The current API operates on a single command, this means that the SYCL-Graph update(std::vector<nodes>) API needs to serialize the list into N calls to the UR API. Given that both OpenCL clUpdateMutableCommandsKHR and Level-Zero
zeCommandListUpdateMutableCommandsExp can operate on a list of commands, this serialization at the UR layer of the stack introduces extra host API calls.

This PR improves the urCommandBufferUpdateKernelLaunchExp API so that a list of commands is passed all the way from SYCL to the native backend API.

As highlighted in oneapi-src/unified-runtime#2671 this patch requires the handle translation auto generated code to be able to handle a list of structs, which is not currently the case. This is PR includes a API specific temporary workaround in the mako file which will unblock this PR until a more permanent solution is completed.

Co-authored-by: Ross Brunton [email protected]

@EwanC EwanC force-pushed the ewan/UR_update_list branch from edd0ae1 to 099e2b3 Compare February 17, 2025 10:51
@EwanC EwanC force-pushed the ewan/UR_update_list branch from 099e2b3 to 81e25a9 Compare February 18, 2025 10:57
@EwanC EwanC force-pushed the ewan/UR_update_list branch from 81e25a9 to 0176505 Compare February 19, 2025 10:05
@EwanC EwanC changed the title [SYCL][Graph] Batch graph updates [SYCL][Graph][UR] Propagate graph update list to UR Feb 19, 2025
@EwanC EwanC force-pushed the ewan/UR_update_list branch from 0176505 to 1fd571d Compare February 19, 2025 12:46
@EwanC EwanC force-pushed the ewan/UR_update_list branch from fb60d96 to c8a5fa8 Compare February 24, 2025 14:31
@EwanC EwanC marked this pull request as ready for review February 24, 2025 15:05
@EwanC EwanC requested review from a team as code owners February 24, 2025 15:05
Copy link
Contributor

@aelovikov-intel aelovikov-intel left a comment

Choose a reason for hiding this comment

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

I think SYCL RT's approval is just a formality here, graphs reviewers is what matters.

EwanC and others added 2 commits February 26, 2025 12:29
Update the `urCommandBufferUpdateKernelLaunchExp` API for updating commands in a command-buffer to take a list of commands.

The current API operates on a single command, this means that the SYCL-Graph `update(std::vector<nodes>)` API needs to serialize the list into N calls to the UR API. Given that both OpenCL `clUpdateMutableCommandsKHR` and Level-Zero
`zeCommandListUpdateMutableCommandsExp` can operate on a list of commands, this serialization at the UR layer of the stack introduces extra host API calls.

This PR improves the `urCommandBufferUpdateKernelLaunchExp` API so that a list of commands is passed all the way from SYCL to the native backend API.

As highlighted in oneapi-src/unified-runtime#2671 this patch requires the handle translation auto generated code to be able to handle a list of structs, which is not currently the case. This is PR includes a API specific temporary workaround in the mako file which will unblock this PR until a more permanent solution is completed.

Co-authored-by: Ross Brunton <[email protected]>
@EwanC
Copy link
Contributor Author

EwanC commented Mar 4, 2025

@intel/llvm-gatekeepers This is ready to merge, thanks

@sommerlukas sommerlukas merged commit db7eac4 into intel:sycl Mar 4, 2025
30 checks passed
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.

5 participants