Skip to content

Conversation

@EwanC
Copy link
Contributor

@EwanC EwanC commented Aug 12, 2024

As discovered in #1951 (comment) there is an issue running the command-buffer CTS tests on L0 with command-lists enabled.

After investigating, this was because the CTS tests we're not using the output event parameter to command-buffer enqueue. In the L0 adapter code a path for registering an event with the queue to waiting on the workload was guarded by this event being set. Therefore urQueueFinish was not working as expected, as the queue was not aware of this work to wait on.

Fixed by always registering the work to wait on with the queue, and miss out on propagation of the created event when user doesn't pass an output event, rather than not creating it at all.

DPC++ PR intel/llvm#15035

As discovered in
oneapi-src#1951 (comment)
there is an issue running the command-buffer CTS tests on L0 with
command-lists enabled.

After investigating, this was because the CTS tests we're not using the
output event parameter to command-buffer enqueue. In the L0 adapter
code a path for registering an event with the queue to waiting on
the workload was guarded by this event being set. Therefore
`urQueueFinish` was not working as expected, as the queue was not aware
of this work to wait on.

Fixed by always registering the work to wait on with the queue, and
miss out on propagation of the created event when user doesn't pass
an output event, rather than not creating it at all.
@EwanC EwanC marked this pull request as ready for review August 12, 2024 09:50
@EwanC EwanC requested a review from a team as a code owner August 12, 2024 09:50
@github-actions github-actions bot added level-zero L0 adapter specific issues command-buffer Command Buffer feature addition/changes/specification labels Aug 12, 2024
@EwanC EwanC added the v0.10.x Include in the v0.10.x release label Aug 12, 2024
@EwanC EwanC added the ready to merge Added to PR's which are ready to merge label Aug 12, 2024
Copy link
Contributor

@nrspruit nrspruit left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for the fix!

@omarahmed1111 omarahmed1111 merged commit 26f1dfc into oneapi-src:main Aug 13, 2024
omarahmed1111 pushed a commit to reble/llvm that referenced this pull request Aug 13, 2024
Tests UR PR oneapi-src/unified-runtime#1960
for fixing a UR CTS bug in the L0 adapter for command-buffers.
steffenlarsen pushed a commit to intel/llvm that referenced this pull request Aug 13, 2024
Tests UR PR oneapi-src/unified-runtime#1960 for
fixing a UR CTS bug in the L0 adapter for command-buffers.
@kbenzie kbenzie mentioned this pull request Aug 13, 2024
53 tasks
kbenzie pushed a commit that referenced this pull request Aug 13, 2024
Command-buffer immediate command-list CTS fix
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 level-zero L0 adapter specific issues ready to merge Added to PR's which are ready to merge v0.10.x Include in the v0.10.x release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants