Skip to content

Conversation

@EwanC
Copy link
Contributor

@EwanC EwanC commented Dec 12, 2024

UR program and kernel objects can be tied to multiple devices, a UR command-buffer object however is tied to a single device.

When appending a kernel command to a command-buffer, select the correct single-device ze_kernel_handle_t object from the multi-device ur_kernel_handle_t object

DPC++ PR intel/llvm#16343

@EwanC EwanC added the v0.11.x Include in the v0.11.x release label Dec 12, 2024
@github-actions github-actions bot added level-zero L0 adapter specific issues command-buffer Command Buffer feature addition/changes/specification labels Dec 12, 2024
EwanC pushed a commit to reble/llvm that referenced this pull request Dec 12, 2024
Bumps UR commit to oneapi-src/unified-runtime#2454
which contains a fix for using kernel bundles associated
with a multi-device context in a SYCL-Graph
@EwanC EwanC force-pushed the l0_cmd-buf_multi-device branch from 0b0295c to f0facdb Compare December 12, 2024 15:51
@github-actions github-actions bot added the conformance Conformance test suite issues. label Dec 12, 2024
@EwanC EwanC force-pushed the l0_cmd-buf_multi-device branch 2 times, most recently from 434cf05 to 583af28 Compare December 12, 2024 16:42
@EwanC EwanC marked this pull request as ready for review December 12, 2024 17:04
@EwanC EwanC requested review from a team as code owners December 12, 2024 17:04
@EwanC EwanC requested a review from fabiomestre December 12, 2024 17:04
@EwanC EwanC force-pushed the l0_cmd-buf_multi-device branch from 583af28 to 352af52 Compare December 12, 2024 20:39

static bool hasCommandBufferSupport(ur_device_handle_t device) {
size_t returned_size;
auto res = urDeviceGetInfo(device, UR_DEVICE_INFO_EXTENSIONS, 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

can we use UR_DEVICE_INFO_COMMAND_BUFFER_SUPPORT_EXP here instead?

Copy link
Contributor Author

@EwanC EwanC Dec 13, 2024

Choose a reason for hiding this comment

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

yes, updated (really need to get #2304 merged 😉 )

Copy link
Contributor

@kbenzie kbenzie left a comment

Choose a reason for hiding this comment

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

CTS changes LGTM

};

TEST_F(urMultiDeviceCommandBufferExpTest, Enqueue) {
for (size_t i = 0; i < devices.size(); i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@aarongreig FYI these test will need updated in your CTS multi-platform/device branch.

@kbenzie kbenzie added the ready to merge Added to PR's which are ready to merge label Dec 13, 2024
UR program and kernel objects can be tied to multiple devices,
a UR command-buffer object however is tied to a single device.

When appending a kernel command to a command-buffer, select the
correct single-device `ze_kernel_handle_t` object from the multi-device
`ur_kernel_handle_t` object
@EwanC EwanC force-pushed the l0_cmd-buf_multi-device branch from 352af52 to fcddf07 Compare December 13, 2024 11:21
@martygrant martygrant merged commit b7047f6 into oneapi-src:main Dec 13, 2024
73 checks passed
martygrant added a commit to intel/llvm that referenced this pull request Dec 13, 2024
Bumps UR commit to
oneapi-src/unified-runtime#2454 which contains a
fix for using kernel bundles associated with a multi-device context in a
SYCL-Graph

---------

Co-authored-by: Martin Morrison-Grant <[email protected]>
kbenzie pushed a commit that referenced this pull request Dec 16, 2024
Fix L0 command-buffer consumption of multi-device kernels
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. level-zero L0 adapter specific issues ready to merge Added to PR's which are ready to merge v0.11.x Include in the v0.11.x release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants