[SYCL] Scheduler and scheduler-bypass path for handler-less barriers#21729
[SYCL] Scheduler and scheduler-bypass path for handler-less barriers#21729slawekptak wants to merge 12 commits intointel:syclfrom
Conversation
slawekptak
changed the title
slawekptak
changed the title
There was a problem hiding this comment.
Pull request overview
This PR updates SYCL queue barrier submission to support handler-less barriers via both the scheduler path and a scheduler-bypass (“direct”) path, and adds unit tests to validate ordering and fast-path behavior.
Changes:
- Route
queue::ext_oneapi_submit_barrier(...)overloads through a unifiedqueue_impl::submit_barrier_direct_impl(...)with explicitCGType(BarriervsBarrierWaitlist). - Add a new scheduler-bypass implementation for barriers (
submit_barrier_scheduler_bypass) and adjust dependency tracking insubmit_direct. - Add scheduler unit tests covering host-task vs barrier ordering and the empty-waitlist shortcut.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| sycl/source/queue.cpp | Switch barrier API overloads to call the new direct barrier submission entrypoint with explicit CGType. |
| sycl/source/detail/queue_impl.hpp | Extend barrier direct submission APIs to carry CGType and declare the new bypass helper. |
| sycl/source/detail/queue_impl.cpp | Implement scheduler-bypass barrier submission and refactor direct barrier submission to pick scheduler vs bypass. |
| sycl/unittests/scheduler/InOrderQueueHostTaskDeps.cpp | Add a test ensuring an in-order host task blocks barrier enqueueing. |
| sycl/unittests/scheduler/BarrierDependencies.cpp | Add a test intended to validate the empty-waitlist shortcut behavior. |
| if (BarrierType == CGType::Barrier) { | ||
| if (RawDepEvents.size()) { | ||
| getAdapter().call<UrApiKind::urEnqueueEventsWait>( | ||
| getHandleRef(), RawDepEvents.size(), &RawDepEvents[0], nullptr); | ||
| } | ||
|
|
||
| getAdapter().call<UrApiKind::urEnqueueEventsWaitWithBarrierExt>( | ||
| getHandleRef(), nullptr, 0, nullptr, &UREvent); | ||
| } else { | ||
|
|
||
| RawDepEvents.insert(RawDepEvents.end(), RawBarrierDepEvents.begin(), | ||
| RawBarrierDepEvents.end()); | ||
|
|
||
| getAdapter().call<UrApiKind::urEnqueueEventsWaitWithBarrierExt>( | ||
| getHandleRef(), &Properties, UrDepEvents.size(), | ||
| UrDepEvents.size() ? UrDepEvents.data() : nullptr, &UREvent); | ||
| getAdapter().call<UrApiKind::urEnqueueEventsWaitWithBarrierExt>( | ||
| getHandleRef(), nullptr, RawDepEvents.size(), RawDepEvents.data(), | ||
| &UREvent); |
There was a problem hiding this comment.
submit_barrier_scheduler_bypass calls urEnqueueEventsWaitWithBarrierExt with a null ur_exp_enqueue_ext_properties_t* (passing nullptr). In the scheduler path we always pass a properly initialized properties struct (with stype = UR_STRUCTURE_TYPE_EXP_ENQUEUE_EXT_PROPERTIES), and UR implementations may assume this pointer is non-null. Please build and pass an initialized ur_exp_enqueue_ext_properties_t (even if flags = 0) to both calls here to avoid UB / backend-specific failures.
| InOrderQueue.wait(); | ||
|
|
There was a problem hiding this comment.
This test doesn’t currently validate the intended “empty queue shortcut”: it calls InOrderQueue.wait() and then asserts the event is complete, which would be true even if the barrier was actually enqueued and executed normally. To cover the shortcut behavior, assert completion without waiting (or attach a UR mock callback and assert urEnqueueEventsWaitWithBarrierExt is not called when the waitlist is empty).
| InOrderQueue.wait(); |
| // But, if all events in 'Events' can be skipped (NOP or host events), | ||
| // then the barrier itself can be skipped as well. | ||
| if (!DepEvents.empty() && UrDepEvents.empty()) { | ||
| // TODO Do we have to honor the "depends_on" dependencies here? |
There was a problem hiding this comment.
I think yes, don't we do it in the existing scheduler path?
properties in the queue shortcut barrier methods.
No description provided.