Skip to content
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

Add support for command-buffer kernel updates #1924

Merged
merged 1 commit into from
Sep 30, 2024

Conversation

fabiomestre
Copy link
Contributor

@fabiomestre fabiomestre commented Aug 2, 2024

  • Updates the specification to add support for command-buffer kernel handle updates.
  • Adds new UR tests for this feature.
  • Adds an implementation for the Cuda and Hip adapters.
  • Changes the UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_SUPPORT_EXP enum to a new UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_CAPABILITIES_EXP which uses a bitfield instead of a boolean.
  • Changes the spec for urCommandBufferUpdateKernelLaunchExp and ur_exp_command_buffer_update_kernel_launch_desc_t to make it more intuitive with less complicated errors codes:
    • Passing a non-nullptr local work-group will now always update the command to use that value.
    • Passing a nullptr local work-group will now always keep the current command behaviour (either generated by the implementation or user-defined).
    • Passing zero to newWorkDim is now an error.

intel/llvm PR: intel/llvm#15287

@github-actions github-actions bot added loader Loader related feature/bug conformance Conformance test suite issues. specification Changes or additions to the specification experimental Experimental feature additions/changes/specification level-zero L0 adapter specific issues cuda CUDA adapter specific issues hip HIP adapter specific issues native-cpu Native CPU adapter specific issues command-buffer Command Buffer feature addition/changes/specification labels Aug 2, 2024
scripts/core/exp-command-buffer.yml Outdated Show resolved Hide resolved
scripts/core/EXP-COMMAND-BUFFER.rst Show resolved Hide resolved
scripts/core/EXP-COMMAND-BUFFER.rst Outdated Show resolved Hide resolved
test/conformance/exp_enqueue_native/CMakeLists.txt Outdated Show resolved Hide resolved
scripts/core/exp-command-buffer.yml Outdated Show resolved Hide resolved
source/adapters/cuda/command_buffer.cpp Show resolved Hide resolved
source/adapters/hip/command_buffer.cpp Outdated Show resolved Hide resolved
test/conformance/exp_command_buffer/fixtures.h Outdated Show resolved Hide resolved
scripts/core/EXP-COMMAND-BUFFER.rst Show resolved Hide resolved
@EwanC EwanC changed the title [SPEC] Add support for command-buffer kernel updates Add support for command-buffer kernel updates Sep 5, 2024
@fabiomestre fabiomestre force-pushed the fabio/cmd_buffer_kernel_update branch 2 times, most recently from 9997afa to f478846 Compare September 10, 2024 19:56
scripts/core/exp-command-buffer.yml Outdated Show resolved Hide resolved
scripts/core/exp-command-buffer.yml Outdated Show resolved Hide resolved
scripts/core/exp-command-buffer.yml Outdated Show resolved Hide resolved
scripts/core/exp-command-buffer.yml Outdated Show resolved Hide resolved
@fabiomestre fabiomestre marked this pull request as ready for review September 11, 2024 14:44
@fabiomestre fabiomestre requested review from a team as code owners September 11, 2024 14:44
scripts/core/exp-command-buffer.yml Outdated Show resolved Hide resolved
source/adapters/opencl/common.cpp Outdated Show resolved Hide resolved
scripts/core/exp-command-buffer.yml Outdated Show resolved Hide resolved
@fabiomestre fabiomestre force-pushed the fabio/cmd_buffer_kernel_update branch 4 times, most recently from 5ddaec0 to 59e890c Compare September 17, 2024 20:10
@fabiomestre fabiomestre force-pushed the fabio/cmd_buffer_kernel_update branch 2 times, most recently from 7735328 to 52f534d Compare September 18, 2024 12:23
@fabiomestre
Copy link
Contributor Author

This PR should be ready for reviewing:
@oneapi-src/unified-runtime-native-cpu-write @oneapi-src/unified-runtime-opencl-write @oneapi-src/unified-runtime-hip-write @oneapi-src/unified-runtime-cuda-write @oneapi-src/unified-runtime-level-zero-write @oneapi-src/unified-runtime-maintain

Copy link
Contributor

@PietroGhg PietroGhg left a comment

Choose a reason for hiding this comment

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

Native CPU lgtm, thank you

Copy link
Contributor

@JackAKirk JackAKirk left a comment

Choose a reason for hiding this comment

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

cuda/hip lgtm

- Updates the specification to add support for command-buffer kernel handle updates.
- Adds new UR tests for this feature.
- Adds an implementation for the Cuda and Hip adapters.
- Changes the UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_SUPPORT_EXP enum to a new UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_CAPABILITIES_EXP which uses a bitfield instead of a boolean.
- Changes the spec for urCommandBufferUpdateKernelLaunchExp and ur_exp_command_buffer_update_kernel_launch_desc_t to make it more intuitive with less complicated errors codes:
    - Passing a non-nullptr local work-group will now always update the command to use that value.
    - Passing a nullptr local work-group will now always keep the current command behaviour (either generated by the implementation or user-defined).
    - Passing zero to newWorkDim is now an error.
@EwanC EwanC added the ready to merge Added to PR's which are ready to merge label Sep 27, 2024
@aarongreig aarongreig merged commit 532a4ec into oneapi-src:main Sep 30, 2024
142 of 148 checks passed
sarnex pushed a commit to intel/llvm that referenced this pull request Oct 1, 2024
…15287)

Updates the call to urCommandBufferAppendKernelLaunchExp to use the new
UR parameters.

Corresponding UR PR:
oneapi-src/unified-runtime#1924

---------

Co-authored-by: Aaron Greig <aaron.greig@codeplay.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
command-buffer Command Buffer feature addition/changes/specification conformance Conformance test suite issues. cuda CUDA adapter specific issues experimental Experimental feature additions/changes/specification hip HIP adapter specific issues level-zero L0 adapter specific issues loader Loader related feature/bug native-cpu Native CPU adapter specific issues ready to merge Added to PR's which are ready to merge specification Changes or additions to the specification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants