-
Notifications
You must be signed in to change notification settings - Fork 797
[UR][Offload] Event waiting #19594
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
base: sycl
Are you sure you want to change the base?
[UR][Offload] Event waiting #19594
Conversation
Implement urEventsWait[WithBarrier] and respect the waitlist of enqueue functions.
|
||
// Create events on each active queue for an arbitrary thread to block on | ||
// TODO: Can we efficiently check if each thread is "finished" rather than | ||
// creating an event? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stream_queue_t::syncEvents
has some optimizations to only sync streams that need to be
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stream_queue_t::syncEvents
doesn't seem to exist. stream_queue_t::syncStreams
does, but doesn't seem to exclude completed threads as far as I can tell.
} | ||
|
||
// Ensure any newly created work waits on this barrier | ||
auto OldEvent = hQueue->Barrier.exchange(BarrierEvent); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if exchange failed, that means that a newer barrier was created by a different thread. This means we should probably deallocate BarrierEvent
at the end of this function.
Tbh, I think I'd just use a lock.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this logic is sound when it comes to threading. What pattern are you seeing where it isn't?
Replacing the atomic with a lock feels like it'd slow queuing down quite a bit (since every enqueue would have to acquire the lock). Although, I agree it's much easier to reason about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just decided to switch it over to a mutex.
Implement urEventsWait[WithBarrier] and respect the waitlist of enqueue
functions.