[SYCL] Fix PI event leak in memcpy2d device-host fallback (#8878)

The fallback mechanism for memcpy2d and copy2d between host and device
enqueues a number of memcpy operations to/from device and inserts a
marker event to signify the convergence. Currently the intermediate
events are not correctly released, causing them to leak. This commit
fixes this by utilizing an existing RAII PI event managed object to
manage the lifetime of these events.

---------

Signed-off-by: Larsen, Steffen <[email protected]>

Author: steffenlarsen

sycl/source/detail/device_global_map_entry.cpp

@@ -17,18 +17,6 @@ namespace sycl {
 __SYCL_INLINE_VER_NAMESPACE(_V1) {
 namespace detail {
 
-OwnedPiEvent::OwnedPiEvent(RT::PiEvent Event, const plugin &Plugin)
-    : MEvent(Event), MPlugin(Plugin) {
-  // Retain the event to share ownership of it.
-  MPlugin.call<PiApiKind::piEventRetain>(*MEvent);
-}
-
-OwnedPiEvent::~OwnedPiEvent() {
-  // Release the event if the ownership was not transferred.
-  if (MEvent.has_value())
-    MPlugin.call<PiApiKind::piEventRelease>(*MEvent);
-}
-
 DeviceGlobalUSMMem::~DeviceGlobalUSMMem() {
   // removeAssociatedResources is expected to have cleaned up both the pointer
   // and the event. When asserts are enabled the values are set, so we check

sycl/source/detail/device_global_map_entry.hpp

@@ -15,8 +15,8 @@
 #include <optional>
 #include <set>
 
+#include <detail/pi_utils.hpp>
 #include <sycl/detail/defines_elementary.hpp>
-#include <sycl/detail/pi.hpp>
 
 namespace sycl {
 __SYCL_INLINE_VER_NAMESPACE(_V1) {
@@ -28,38 +28,6 @@ class device_impl;
 class platform_impl;
 class queue_impl;
 
-// RAII object for keeping ownership of a PI event.
-struct OwnedPiEvent {
-  OwnedPiEvent(const plugin &Plugin) : MEvent{std::nullopt}, MPlugin{Plugin} {}
-  OwnedPiEvent(RT::PiEvent Event, const plugin &Plugin);
-  ~OwnedPiEvent();
-
-  OwnedPiEvent(OwnedPiEvent &&Other)
-      : MEvent(Other.MEvent), MPlugin(Other.MPlugin) {
-    Other.MEvent = std::nullopt;
-  }
-
-  // Copy constructor explicitly deleted for simplicity as it is not currently
-  // used. Implement if needed.
-  OwnedPiEvent(const OwnedPiEvent &Other) = delete;
-
-  operator bool() { return MEvent.has_value(); }
-
-  RT::PiEvent GetEvent() { return *MEvent; }
-
-  // Transfers the ownership of the event to the caller. The destructor will
-  // no longer release the event.
-  RT::PiEvent TransferOwnership() {
-    RT::PiEvent Event = *MEvent;
-    MEvent = std::nullopt;
-    return Event;
-  }
-
-private:
-  std::optional<RT::PiEvent> MEvent;
-  const plugin &MPlugin;
-};
-
 struct DeviceGlobalUSMMem {
   DeviceGlobalUSMMem(void *Ptr) : MPtr(Ptr) {}
   ~DeviceGlobalUSMMem();

sycl/source/detail/memory_manager.cpp

@@ -11,6 +11,7 @@
 #include <detail/event_impl.hpp>
 #include <detail/mem_alloc_helper.hpp>
 #include <detail/memory_manager.hpp>
+#include <detail/pi_utils.hpp>
 #include <detail/queue_impl.hpp>
 #include <detail/xpti_registry.hpp>
 
@@ -963,13 +964,18 @@ void MemoryManager::copy_2d_usm(const void *SrcMem, size_t SrcPitch,
 #endif // NDEBUG
 
   // The fallback in this case is to insert a copy per row.
+  std::vector<OwnedPiEvent> CopyEventsManaged;
+  CopyEventsManaged.reserve(Height);
+  // We'll need continuous range of events for a wait later as well.
   std::vector<RT::PiEvent> CopyEvents(Height);
   for (size_t I = 0; I < Height; ++I) {
     char *DstItBegin = static_cast<char *>(DstMem) + I * DstPitch;
     const char *SrcItBegin = static_cast<const char *>(SrcMem) + I * SrcPitch;
     Plugin.call<PiApiKind::piextUSMEnqueueMemcpy>(
         Queue->getHandleRef(), /* blocking */ PI_FALSE, DstItBegin, SrcItBegin,
         Width, DepEvents.size(), DepEvents.data(), CopyEvents.data() + I);
+    CopyEventsManaged.emplace_back(CopyEvents[I], Plugin,
+                                   /*TakeOwnership=*/true);
   }
 
   // Then insert a wait to coalesce the copy events.

sycl/source/detail/pi_utils.hpp

@@ -0,0 +1,66 @@
+//==------------- pi_utils.hpp - Common PI utilities -----------------------==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#pragma once
+
+#include <detail/plugin.hpp>
+#include <sycl/detail/defines_elementary.hpp>
+#include <sycl/detail/pi.hpp>
+
+#include <optional>
+
+namespace sycl {
+__SYCL_INLINE_VER_NAMESPACE(_V1) {
+namespace detail {
+
+// RAII object for keeping ownership of a PI event.
+struct OwnedPiEvent {
+  OwnedPiEvent(const plugin &Plugin) : MEvent{std::nullopt}, MPlugin{Plugin} {}
+  OwnedPiEvent(RT::PiEvent Event, const plugin &Plugin,
+               bool TakeOwnership = false)
+      : MEvent(Event), MPlugin(Plugin) {
+    // If it is not instructed to take ownership, retain the event to share
+    // ownership of it.
+    if (!TakeOwnership)
+      MPlugin.call<PiApiKind::piEventRetain>(*MEvent);
+  }
+  ~OwnedPiEvent() {
+    // Release the event if the ownership was not transferred.
+    if (MEvent.has_value())
+      MPlugin.call<PiApiKind::piEventRelease>(*MEvent);
+  }
+
+  OwnedPiEvent(OwnedPiEvent &&Other)
+      : MEvent(Other.MEvent), MPlugin(Other.MPlugin) {
+    Other.MEvent = std::nullopt;
+  }
+
+  // Copy constructor explicitly deleted for simplicity as it is not currently
+  // used. Implement if needed.
+  OwnedPiEvent(const OwnedPiEvent &Other) = delete;
+
+  operator bool() { return MEvent.has_value(); }
+
+  RT::PiEvent GetEvent() { return *MEvent; }
+
+  // Transfers the ownership of the event to the caller. The destructor will
+  // no longer release the event.
+  RT::PiEvent TransferOwnership() {
+    RT::PiEvent Event = *MEvent;
+    MEvent = std::nullopt;
+    return Event;
+  }
+
+private:
+  std::optional<RT::PiEvent> MEvent;
+  const plugin &MPlugin;
+};
+
+} // namespace detail
+} // __SYCL_INLINE_VER_NAMESPACE(_V1)
+} // namespace sycl