Skip to content

Commit 8ba9fb4

Browse files
committed
[SYCL] Fix handling of discarded events in preview lib
The queue implementation assumed that handler.finalize() will return either a valid (non-discarded) event or nullptr (that's why parseEvent is nop in preview mode). However, that was not always true. It was possible for scheduler to only mark an event as discarded after handler.finalize() completed (during actual command execution). This resulted in discarded event being stored in LastEventPtr in the queue and calls to wait() on that discarded event (which is not allowed) in MT scenarios. Fix this, by modyfing addCG() to mark the event as discarded immediately. This allows handler to return nullptr for preview mode.
1 parent 3b13547 commit 8ba9fb4

File tree

4 files changed

+21
-15
lines changed

4 files changed

+21
-15
lines changed

sycl/source/detail/scheduler/commands.cpp

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3132,22 +3132,17 @@ ur_result_t ExecCGCommand::enqueueImpQueue() {
31323132
auto RawEvents = getUrEvents(EventImpls);
31333133
flushCrossQueueDeps(EventImpls, MWorkerQueue);
31343134

3135-
// We can omit creating a UR event and create a "discarded" event if the
3136-
// command has been explicitly marked as not needing an event, e.g. if the
3137-
// user did not ask for one, and there are no requirements.
3138-
bool DiscardUrEvent = MQueue && !MEventNeeded &&
3139-
MQueue->supportsDiscardingPiEvents() &&
3140-
MCommandGroup->getRequirements().size() == 0;
3141-
31423135
ur_event_handle_t UREvent = nullptr;
3143-
ur_event_handle_t *Event = DiscardUrEvent ? nullptr : &UREvent;
3144-
detail::event_impl *EventImpl = DiscardUrEvent ? nullptr : MEvent.get();
3136+
ur_event_handle_t *Event = !MEventNeeded ? nullptr : &UREvent;
3137+
detail::event_impl *EventImpl = !MEventNeeded ? nullptr : MEvent.get();
31453138

31463139
auto SetEventHandleOrDiscard = [&]() {
3147-
if (Event)
3140+
if (!MEventNeeded) {
3141+
assert(MEvent->isDiscarded());
3142+
} else {
3143+
assert(!MEvent->isDiscarded());
31483144
MEvent->setHandle(*Event);
3149-
else
3150-
MEvent->setStateDiscarded();
3145+
}
31513146
};
31523147

31533148
switch (MCommandGroup->getType()) {

sycl/source/detail/scheduler/scheduler.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,9 @@ EventImplPtr Scheduler::addCG(
133133
}
134134
NewEvent = NewCmd->getEvent();
135135
NewEvent->setSubmissionTime();
136+
if (!EventNeeded) {
137+
NewEvent->setStateDiscarded();
138+
}
136139
}
137140

138141
enqueueCommandForCG(NewEvent, AuxiliaryCmds);

sycl/source/detail/scheduler/scheduler.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,8 @@ class Scheduler {
376376
/// directly to the queue.
377377
/// \param Dependencies Optional list of dependency
378378
/// sync points when enqueuing to a command buffer.
379-
/// \return an event object to wait on for command group completion.
379+
/// \return an event object to wait on for command group completion. It can
380+
/// be a discarded event.
380381
EventImplPtr addCG(
381382
std::unique_ptr<detail::CG> CommandGroup, const QueueImplPtr &Queue,
382383
bool EventNeeded, ur_exp_command_buffer_handle_t CommandBuffer = nullptr,

sycl/source/handler.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -895,11 +895,18 @@ event handler::finalize() {
895895
#endif
896896
}
897897

898+
bool DiscardEvent = !impl->MEventNeeded && Queue &&
899+
Queue->supportsDiscardingPiEvents() &&
900+
CommandGroup->getRequirements().size() == 0;
901+
898902
detail::EventImplPtr Event = detail::Scheduler::getInstance().addCG(
899-
std::move(CommandGroup), Queue->shared_from_this(), impl->MEventNeeded);
903+
std::move(CommandGroup), Queue->shared_from_this(), !DiscardEvent);
900904

901905
#ifdef __INTEL_PREVIEW_BREAKING_CHANGES
902-
MLastEvent = Event;
906+
// For preview mode, handler.finalize() is expected to return nullptr
907+
// if the event is discarded.
908+
MLastEvent = DiscardEvent ? nullptr : Event;
909+
assert(MLastEvent || !MLastEvent->isDiscarded());
903910
#else
904911
MLastEvent = detail::createSyclObjFromImpl<event>(Event);
905912
#endif

0 commit comments

Comments
 (0)