[SYCL][L0] Rework active barriers tracking to achieve correct reference counting (#7893)

Signed-off-by: Sergey V Maslov <[email protected]>

Author: smaslov-intel

sycl/plugins/level_zero/pi_level_zero.cpp

@@ -2004,27 +2004,38 @@ pi_result _pi_queue::insertActiveBarriers(pi_command_list_ptr_t &CmdList,
   if (ActiveBarriers.empty())
     return PI_SUCCESS;
 
-  // Create a wait-list and retain events. This will filter out finished events.
+  // Create a wait-list and retain events.
   _pi_ze_event_list_t ActiveBarriersWaitList;
   if (auto Res = ActiveBarriersWaitList.createAndRetainPiZeEventList(
-          ActiveBarriers.size(), ActiveBarriers.data(), this, UseCopyEngine))
+          ActiveBarriers.vector().size(), ActiveBarriers.vector().data(), this,
+          UseCopyEngine))
     return Res;
 
-  // We can now release all the active barriers and replace them with the ones
-  // in the wait list.
-  for (pi_event &BarrierEvent : ActiveBarriers)
-    PI_CALL(piEventReleaseInternal(BarrierEvent));
+  // We can now replace active barriers with the ones in the wait list.
   ActiveBarriers.clear();
-  ActiveBarriers.insert(
-      ActiveBarriers.end(), ActiveBarriersWaitList.PiEventList,
-      ActiveBarriersWaitList.PiEventList + ActiveBarriersWaitList.Length);
+
+  if (ActiveBarriersWaitList.Length == 0) {
+    return PI_SUCCESS;
+  }
+
+  for (pi_uint32 I = 0; I < ActiveBarriersWaitList.Length; ++I) {
+    auto &Event = ActiveBarriersWaitList.PiEventList[I];
+    ActiveBarriers.add(Event);
+  }
+
+  pi_event Event = nullptr;
+  if (auto Res = createEventAndAssociateQueue(
+          this, &Event, PI_COMMAND_TYPE_USER, CmdList, /*IsInternal*/ true))
+    return Res;
+
+  Event->WaitList = ActiveBarriersWaitList;
+  Event->OwnZeEvent = true;
 
   // If there are more active barriers, insert a barrier on the command-list. We
   // do not need an event for finishing so we pass nullptr.
-  if (!ActiveBarriers.empty())
-    ZE_CALL(zeCommandListAppendBarrier,
-            (CmdList->first, nullptr, ActiveBarriersWaitList.Length,
-             ActiveBarriersWaitList.ZeEventList));
+  ZE_CALL(zeCommandListAppendBarrier,
+          (CmdList->first, nullptr, ActiveBarriersWaitList.Length,
+           ActiveBarriersWaitList.ZeEventList));
   return PI_SUCCESS;
 }
 
@@ -6191,6 +6202,17 @@ pi_result piEventRelease(pi_event Event) {
   return PI_SUCCESS;
 }
 
+void _pi_queue::active_barriers::add(pi_event &Event) {
+  Event->RefCount.increment();
+  Events.push_back(Event);
+}
+
+void _pi_queue::active_barriers::clear() {
+  for (const auto &Event : Events)
+    piEventReleaseInternal(Event);
+  Events.clear();
+}
+
 static pi_result piEventReleaseInternal(pi_event Event) {
   PI_ASSERT(Event, PI_ERROR_INVALID_EVENT);
 
@@ -6561,6 +6583,7 @@ pi_result piEnqueueEventsWaitWithBarrier(pi_queue Queue,
         if (auto Res = createEventAndAssociateQueue(
                 Queue, &Event, PI_COMMAND_TYPE_USER, CmdList, IsInternal))
           return Res;
+
         Event->WaitList = EventWaitList;
         ZE_CALL(zeCommandListAppendBarrier,
                 (CmdList->first, Event->ZeEvent, EventWaitList.Length,
@@ -6611,21 +6634,13 @@ pi_result piEnqueueEventsWaitWithBarrier(pi_queue Queue,
     // Because of the dependency between commands in the in-order queue we don't
     // need to keep track of any active barriers if we have in-order queue.
     if (UseMultipleCmdlistBarriers && !Queue->isInOrderQueue()) {
-      // Retain and save the resulting event for future commands.
-      (*Event)->RefCount.increment();
-      Queue->ActiveBarriers.push_back(*Event);
+      Queue->ActiveBarriers.add(*Event);
     }
     return PI_SUCCESS;
   }
 
   // Since there are no events to explicitly create a barrier for, we are
-  // inserting a queue-wide barrier. As such, the barrier will also encapsulate
-  // the active barriers, so we can release and clear the active barriers list.
-  // Doing it early prevents potential additional barriers from implicitly being
-  // appended.
-  for (pi_event &E : Queue->ActiveBarriers)
-    PI_CALL(piEventReleaseInternal(E));
-  Queue->ActiveBarriers.clear();
+  // inserting a queue-wide barrier.
 
   // Command list(s) for putting barriers.
   std::vector<pi_command_list_ptr_t> CmdLists;
@@ -6688,11 +6703,12 @@ pi_result piEnqueueEventsWaitWithBarrier(pi_queue Queue,
     // Insert a barrier into each unique command queue using the available
     // command-lists.
     std::vector<pi_event> EventWaitVector(CmdLists.size());
-    for (size_t I = 0; I < CmdLists.size(); ++I)
-      if (auto Res = insertBarrierIntoCmdList(
-              CmdLists[I], _pi_ze_event_list_t{}, EventWaitVector[I], false))
+    for (size_t I = 0; I < CmdLists.size(); ++I) {
+      if (auto Res =
+              insertBarrierIntoCmdList(CmdLists[I], _pi_ze_event_list_t{},
+                                       EventWaitVector[I], /*IsInternal*/ true))
         return Res;
-
+    }
     // If there were multiple queues we need to create a "convergence" event to
     // be our active barrier. This convergence event is signalled by a barrier
     // on all the events from the barriers we have inserted into each queue.
@@ -6706,8 +6722,6 @@ pi_result piEnqueueEventsWaitWithBarrier(pi_queue Queue,
             EventWaitVector.size(), EventWaitVector.data(), Queue,
             ConvergenceCmdList->second.isCopy(Queue)))
       return Res;
-    for (pi_event &E : EventWaitVector)
-      PI_CALL(piEventReleaseInternal(E));
 
     // Insert a barrier with the events from each command-queue into the
     // convergence command list. The resulting event signals the convergence of
@@ -6729,9 +6743,8 @@ pi_result piEnqueueEventsWaitWithBarrier(pi_queue Queue,
     if (auto Res = Queue->executeCommandList(CmdList, false, OkToBatch))
       return Res;
 
-  // We must keep the event internally to use if new command lists are created.
-  (*Event)->RefCount.increment();
-  Queue->ActiveBarriers.push_back(*Event);
+  Queue->ActiveBarriers.clear();
+  Queue->ActiveBarriers.add(*Event);
   return PI_SUCCESS;
 }
 
@@ -6838,10 +6851,7 @@ pi_result _pi_queue::synchronize() {
 
   // With the entire queue synchronized, the active barriers must be done so we
   // can remove them.
-  for (pi_event &BarrierEvent : ActiveBarriers)
-    PI_CALL(piEventReleaseInternal(BarrierEvent));
   ActiveBarriers.clear();
-
   return PI_SUCCESS;
 }
 

sycl/plugins/level_zero/pi_level_zero.hpp

@@ -797,10 +797,19 @@ struct _pi_queue : _pi_object {
   pi_result insertActiveBarriers(pi_command_list_ptr_t &CmdList,
                                  bool UseCopyEngine);
 
+  // A helper structure to keep active barriers of the queue.
+  // It additionally manages ref-count of events in this list.
+  struct active_barriers {
+    std::vector<pi_event> Events;
+    void add(pi_event &Event);
+    void clear();
+    bool empty() { return Events.empty(); }
+    std::vector<pi_event> &vector() { return Events; }
+  };
   // A collection of currently active barriers.
   // These should be inserted into a command list whenever an available command
   // list is needed for a command.
-  std::vector<pi_event> ActiveBarriers;
+  active_barriers ActiveBarriers;
 
   // Besides each PI object keeping a total reference count in
   // _pi_object::RefCount we keep special track of the queue *external*

sycl/plugins/unified_runtime/ur/ur.hpp

@@ -121,7 +121,7 @@ template <class T> struct ZeCache : private T {
 
   // Access to the fields of the original T data structure.
   T *operator->() {
-    std::call_once(Computed, Compute, static_cast<T&>(*this));
+    std::call_once(Computed, Compute, static_cast<T &>(*this));
     return this;
   }
 };