Skip to content

[SYCL][HIP] Enable hip_piEnqueueEventsWaitWithBarrier #4975

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 2 commits into from
Jan 10, 2022

Conversation

npmiller
Copy link
Contributor

This patch enables piEnqueueEventsWaitWithBarrier for the HIP plugin, it
brings it to the same level as the CUDA plugin, see #3932.

This fixes the llvm-test-suite, SYCL/Basic/submit_barrier.cpp,
SYCL/Basic/enqueue_barrier.cpp and SYCL/Basic/barrier_order.cpp
tests for the HIP plugin.

This patch enables piEnqueueEventsWaitWithBarrier for the HIP plugin, it
brings it to the same level as the CUDA plugin, see intel#3932.

This fixes the `llvm-test-suite`, `SYCL/Basic/submit_barrier.cpp`,
`SYCL/Basic/enqueue_barrier.cpp` and `SYCL/Basic/barrier_order.cpp`
tests for the HIP plugin.
npmiller added a commit to npmiller/llvm-test-suite that referenced this pull request Nov 17, 2021
command_queue, num_events_in_wait_list, event_wait_list, event);
}

pi_result hip_piEnqueueEventsWaitWithBarrier(pi_queue command_queue,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it implemented the same as hip_piEnqueueEventsWait?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is because at the moment the queue is represented by just a single in order stream, so effectively every command has an implicit barrier and these two functions end up doing the same thing.

This may need to be changed in the future if we improve how the queue and events are implemented, but at the moment it should work fine. This is also the same reason why the CUDA plugin implements these two the same way as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a source code comment about it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated with some comments

@npmiller npmiller requested a review from a team as a code owner January 3, 2022 14:00
@bader bader merged commit 53f1cce into intel:sycl Jan 10, 2022
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
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.

3 participants