Skip to content

[SYCL] Don't return last event in ext_oneapi_submit_barrier #20159

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.

Sign up for GitHub

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 5 commits into from
Sep 24, 2025
Merged

[SYCL] Don't return last event in ext_oneapi_submit_barrier #20159

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e108e6c
[SYCL] Don't return last event in ext_oneapi_submit_barrier
uditagarwal97 Sep 22, 2025
3e3507a
Update test
uditagarwal97 Sep 22, 2025
9620fff
Remove empty queue test
uditagarwal97 Sep 22, 2025
eddffb3
Filter out empty events.
uditagarwal97 Sep 24, 2025
0060d9a
Disable in_order_ext_oneapi_submit_barrier test in 6.3 ABI testing
uditagarwal97 Sep 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 0 additions & 29 deletions sycl/source/queue.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,24 +334,6 @@ void queue::wait_and_throw_proxy(const detail::code_location &CodeLoc) {
impl->wait_and_throw(CodeLoc);
}

static event
getBarrierEventForInorderQueueHelper(detail::queue_impl &QueueImpl) {
// This function should not be called when a queue is recording to a graph,
// as a graph can record from multiple queues and we cannot guarantee the
// last node added by an in-order queue will be the last node added to the
// graph.
assert(!QueueImpl.hasCommandGraph() &&
"Should not be called in on graph recording.");

sycl::detail::optional<event> LastEvent = QueueImpl.getLastEvent();
if (LastEvent)
return *LastEvent;

// If there was no last event, we create an empty one.
return detail::createSyclObjFromImpl<event>(
detail::event_impl::create_default_event());
}

/// Prevents any commands submitted afterward to this queue from executing
/// until all commands previously submitted to this queue have entered the
/// complete state.
Expand All @@ -374,17 +356,6 @@ event queue::ext_oneapi_submit_barrier(const detail::code_location &CodeLoc) {
/// group is being enqueued on.
event queue::ext_oneapi_submit_barrier(const std::vector<event> &WaitList,
const detail::code_location &CodeLoc) {
bool AllEventsEmptyOrNop = std::all_of(
begin(WaitList), end(WaitList), [&](const event &Event) -> bool {
detail::event_impl &EventImpl = *detail::getSyclObjImpl(Event);
return (EventImpl.isDefaultConstructed() || EventImpl.isNOP()) &&
!EventImpl.hasCommandGraph();
});
if (is_in_order() && !impl->hasCommandGraph() && !impl->MIsProfilingEnabled &&
AllEventsEmptyOrNop) {
return getBarrierEventForInorderQueueHelper(*impl);
}

if (WaitList.empty())
return submit([=](handler &CGH) { CGH.ext_oneapi_barrier(); }, CodeLoc);
else
Expand Down
21 changes: 3 additions & 18 deletions sycl/test-e2e/InorderQueue/in_order_ext_oneapi_submit_barrier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,28 +53,13 @@ int main() {
std::cout << "Test 2" << std::endl;
*Res = 0;

auto Event1 = Q.submit(
[&](sycl::handler &CGH) { CGH.host_task([&] { *Res += 1; }); });
auto BarrierEvent1 = Q.ext_oneapi_submit_barrier();
assert(checkBarrierEvent(Q.get_backend(), Event1, BarrierEvent1,
false /* host tasks used */));
auto Event2 = Q.submit([&](sycl::handler &CGH) { CGH.fill(Res, 10, 1); });
Q.submit([&](sycl::handler &CGH) { CGH.host_task([&] { *Res += 1; }); });
Q.ext_oneapi_submit_barrier();
Q.submit([&](sycl::handler &CGH) { CGH.fill(Res, 10, 1); });

Q.wait();
assert(*Res == 10);
}

{
// Test cast 3 - empty queue.
std::cout << "Test 3" << std::endl;
sycl::queue EmptyQ({sycl::property::queue::in_order{}});
auto BarrierEvent = EmptyQ.ext_oneapi_submit_barrier();
assert(
BarrierEvent.get_info<sycl::info::event::command_execution_status>() ==
sycl::info::event_command_status::complete);
BarrierEvent.wait();
}
Comment on lines -67 to -76
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this test removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AlexeySachkov Here's my line of reasoning:
IIUC, for IOQs, ext_submit_barrier() is non-blocking and returns an event, whose state will transition to complete, when all previously submitted commands to IOQ also completes. Now, when we submit a barrier to an empty IOQ, (1) should the implementation immediately return a completed event or (2) can a non-blocking implementation return an event, take some time to figure out if queue is empty or not, and if empty, transition the event to be completed?
The test checks for (1) but IIUC, spec doesn't mandate that.
Looking at UR's implementation of urEnqueueEventsWaitWithBarrierExt, it follows (2) - it returns an event and submits zeCommandListAppendSignalEvent to L0. That's why this test was flakily failing in pre-commit CI of this PR. Before this PR, at SYCL RT level, we check if queue is empty or not and according return a completed event or submit urEnqueueEventsWaitWithBarrierExt. Since getting last event and checking whether queue is empty or not is expensive and (1) is not mandated by spec, I removed the SYCL RT check along with this test.


{
// Test cast 4 - graph.
sycl::queue GQueue{sycl::property::queue::in_order{}};
Expand Down
Loading