[SYCL] Fix handling of discarded events in preview lib (#19005)

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
and to return nullptr is the event is discarded.

---------

Co-authored-by: aelovikov-intel <[email protected]>

Author: igchor

sycl/include/sycl/handler.hpp

@@ -571,6 +571,9 @@ class __SYCL_EXPORT handler {
   /// object destruction.
   ///
   /// \return a SYCL event object representing the command group
+  ///
+  /// Note: in preview mode, handler.finalize() is expected to return
+  /// nullptr if the event is not needed (discarded).
 #ifdef __INTEL_PREVIEW_BREAKING_CHANGES
   detail::EventImplPtr finalize();
 #else

sycl/source/detail/queue_impl.hpp

@@ -712,7 +712,11 @@ class queue_impl : public std::enable_shared_from_this<queue_impl> {
   }
 
 #ifdef __INTEL_PREVIEW_BREAKING_CHANGES
-#define parseEvent(arg) (arg)
+  inline const detail::EventImplPtr &
+  parseEvent(const detail::EventImplPtr &Event) {
+    assert(!Event || !Event->isDiscarded());
+    return Event;
+  }
 #else
   inline detail::EventImplPtr parseEvent(const event &Event) {
     const detail::EventImplPtr &EventImpl = getSyclObjImpl(Event);

sycl/source/detail/scheduler/commands.cpp

@@ -3122,22 +3122,17 @@ ur_result_t ExecCGCommand::enqueueImpQueue() {
   auto RawEvents = getUrEvents(EventImpls);
   flushCrossQueueDeps(EventImpls);
 
-  // We can omit creating a UR event and create a "discarded" event if the
-  // command has been explicitly marked as not needing an event, e.g. if the
-  // user did not ask for one, and there are no requirements.
-  bool DiscardUrEvent = MQueue && !MEventNeeded &&
-                        MQueue->supportsDiscardingPiEvents() &&
-                        MCommandGroup->getRequirements().size() == 0;
-
   ur_event_handle_t UREvent = nullptr;
-  ur_event_handle_t *Event = DiscardUrEvent ? nullptr : &UREvent;
-  detail::event_impl *EventImpl = DiscardUrEvent ? nullptr : MEvent.get();
+  ur_event_handle_t *Event = !MEventNeeded ? nullptr : &UREvent;
+  detail::event_impl *EventImpl = !MEventNeeded ? nullptr : MEvent.get();
 
   auto SetEventHandleOrDiscard = [&]() {
-    if (Event)
+    if (!MEventNeeded) {
+      assert(MEvent->isDiscarded());
+    } else {
+      assert(!MEvent->isDiscarded());
       MEvent->setHandle(*Event);
-    else
-      MEvent->setStateDiscarded();
+    }
   };
 
   switch (MCommandGroup->getType()) {

sycl/source/detail/scheduler/scheduler.cpp

@@ -133,6 +133,14 @@ EventImplPtr Scheduler::addCG(
     }
     NewEvent = NewCmd->getEvent();
     NewEvent->setSubmissionTime();
+
+    // This is the last moment we can mark the event as discarded.
+    // Doing this during command execution would lead to incorrect
+    // event handling (as event would change it's state from non-discarded
+    // to discarded).
+    if (!EventNeeded) {
+      NewEvent->setStateDiscarded();
+    }
   }
 
   enqueueCommandForCG(NewEvent, AuxiliaryCmds);

sycl/source/detail/scheduler/scheduler.hpp

@@ -376,7 +376,8 @@ class Scheduler {
   /// directly to the queue.
   /// \param Dependencies Optional list of dependency
   /// sync points when enqueuing to a command buffer.
-  /// \return an event object to wait on for command group completion.
+  /// \return an event object to wait on for command group completion. It can
+  /// be a discarded event.
   EventImplPtr addCG(
       std::unique_ptr<detail::CG> CommandGroup, const QueueImplPtr &Queue,
       bool EventNeeded, ur_exp_command_buffer_handle_t CommandBuffer = nullptr,

sycl/source/handler.cpp

@@ -894,11 +894,15 @@ event handler::finalize() {
 #endif
   }
 
+  bool DiscardEvent = !impl->MEventNeeded && Queue &&
+                      Queue->supportsDiscardingPiEvents() &&
+                      CommandGroup->getRequirements().size() == 0;
+
   detail::EventImplPtr Event = detail::Scheduler::getInstance().addCG(
-      std::move(CommandGroup), Queue->shared_from_this(), impl->MEventNeeded);
+      std::move(CommandGroup), Queue->shared_from_this(), !DiscardEvent);
 
 #ifdef __INTEL_PREVIEW_BREAKING_CHANGES
-  MLastEvent = Event;
+  MLastEvent = DiscardEvent ? nullptr : Event;
 #else
   MLastEvent = detail::createSyclObjFromImpl<event>(Event);
 #endif