[UR] Replace some UR retains with CL retains

At various points, OpenCL native handles need to be retained to ensure SYCL semantics. Previously, this relied on the fact that UR handles were typecast CL handles and shared the same reference count. However, the SYCL RT shouldn't assume this, so instead we call the appropriate (dynamically looked-up) CL functions on the native handles instead. This is in preperation for oneapi-src/unified-runtime#1176 . This change should also have no observable effect for SYCL code; there is no change in lifetime semantics.
RossBrunton · Feb 14, 2025 · 6b55d56 · 6b55d56
1 parent 254dd3b
commit 6b55d56
Show file tree

Hide file tree

Showing 13 changed files with 60 additions and 27 deletions.
diff --git a/sycl/include/sycl/detail/common.hpp b/sycl/include/sycl/detail/common.hpp
@@ -8,6 +8,9 @@
 
 #pragma once
 
+#ifndef _MSC_VER
+#include <dlfcn.h>
+#endif
 #include <sycl/detail/defines_elementary.hpp> // for __SYCL_ALWAYS_INLINE
 #include <sycl/detail/export.hpp>             // for __SYCL_EXPORT
 
@@ -379,6 +382,27 @@ static constexpr std::array<T, N> RepeatValue(const T &Arg) {
 // classes.
 struct AllowCTADTag;
 
+// Look up a function name that was dynamically linked
+// This is used by the runtime where it needs to manipulate native handles (e.g.
+// retaining OpenCL handles). On Windows, the symbol name is looked up in
+// `winlibname`. In Linux, it uses `RTLD_DEFAULT`. The library must already have
+// been loaded (perhaps by UR).
+template <typename fn>
+fn *dynLookupFunction([[maybe_unused]] const char *winlibname,
+                      const char *name) {
+#ifdef _MSC_VER
+  auto handle = GetModuleHandleA(winlibname);
+  assert(handle);
+  auto *retVal = GetProcAddress(handle, name);
+#else
+  auto *retVal = dlsym(RTLD_DEFAULT, name);
+#endif
+  assert(retVal);
+  return reinterpret_cast<fn *>(retVal);
+}
+#define _OCL_GET_FUNCTION(FN)                                                  \
+  (::sycl::_V1::detail::dynLookupFunction<decltype(FN)>("OpenCL", #FN))
+
 } // namespace detail
 } // namespace _V1
 } // namespace sycl
diff --git a/sycl/source/backend.cpp b/sycl/source/backend.cpp
@@ -181,7 +181,7 @@ __SYCL_EXPORT event make_event(ur_native_handle_t NativeHandle,
       std::make_shared<event_impl>(UrEvent, Context));
 
   if (Backend == backend::opencl)
-    Adapter->call<UrApiKind::urEventRetain>(UrEvent);
+    _OCL_GET_FUNCTION(clRetainEvent)(ur::cast<cl_event>(NativeHandle));
   return Event;
 }
 
@@ -205,7 +205,7 @@ make_kernel_bundle(ur_native_handle_t NativeHandle,
         "urProgramCreateWithNativeHandle resulted in a null program handle.");
 
   if (ContextImpl->getBackend() == backend::opencl)
-    Adapter->call<UrApiKind::urProgramRetain>(UrProgram);
+    _OCL_GET_FUNCTION(clRetainProgram)(ur::cast<cl_program>(NativeHandle));
 
   std::vector<ur_device_handle_t> ProgramDevices;
   uint32_t NumDevices = 0;
@@ -352,7 +352,7 @@ kernel make_kernel(const context &TargetContext,
       &UrKernel);
 
   if (Backend == backend::opencl)
-    Adapter->call<UrApiKind::urKernelRetain>(UrKernel);
+    _OCL_GET_FUNCTION(clRetainKernel)(ur::cast<cl_kernel>(NativeHandle));
 
   // Construct the SYCL queue from UR queue.
   return detail::createSyclObjFromImpl<kernel>(

diff --git a/sycl/source/detail/context_impl.cpp b/sycl/source/detail/context_impl.cpp
@@ -303,10 +303,11 @@ context_impl::findMatchingDeviceImpl(ur_device_handle_t &DeviceUR) const {
 
 ur_native_handle_t context_impl::getNative() const {
   const auto &Adapter = getAdapter();
-  if (getBackend() == backend::opencl)
-    Adapter->call<UrApiKind::urContextRetain>(getHandleRef());
   ur_native_handle_t Handle;
   Adapter->call<UrApiKind::urContextGetNativeHandle>(getHandleRef(), &Handle);
+  if (getBackend() == backend::opencl) {
+    _OCL_GET_FUNCTION(clRetainContext)(ur::cast<cl_context>(Handle));
+  }
   return Handle;
 }
 

diff --git a/sycl/source/detail/device_image_impl.hpp b/sycl/source/detail/device_image_impl.hpp
@@ -300,11 +300,13 @@ class device_image_impl {
     const auto &ContextImplPtr = detail::getSyclObjImpl(MContext);
     const AdapterPtr &Adapter = ContextImplPtr->getAdapter();
 
-    if (ContextImplPtr->getBackend() == backend::opencl)
-      Adapter->call<UrApiKind::urProgramRetain>(MProgram);
     ur_native_handle_t NativeProgram = 0;
     Adapter->call<UrApiKind::urProgramGetNativeHandle>(MProgram,
                                                        &NativeProgram);
+    if (ContextImplPtr->getBackend() == backend::opencl) {
+      auto *RetainFun = _OCL_GET_FUNCTION(clRetainProgram);
+      RetainFun(ur::cast<cl_program>(NativeProgram));
+    }
 
     return NativeProgram;
   }

diff --git a/sycl/source/detail/device_impl.cpp b/sycl/source/detail/device_impl.cpp
@@ -98,7 +98,7 @@ bool device_impl::is_affinity_supported(
 
 cl_device_id device_impl::get() const {
   // TODO catch an exception and put it to list of asynchronous exceptions
-  getAdapter()->call<UrApiKind::urDeviceRetain>(MDevice);
+  _OCL_GET_FUNCTION(clRetainDevice)(ur::cast<cl_device_id>(getNative()));
   return ur::cast<cl_device_id>(getNative());
 }
 
@@ -345,10 +345,11 @@ std::vector<device> device_impl::create_sub_devices() const {
 
 ur_native_handle_t device_impl::getNative() const {
   auto Adapter = getAdapter();
-  if (getBackend() == backend::opencl)
-    Adapter->call<UrApiKind::urDeviceRetain>(getHandleRef());
   ur_native_handle_t Handle;
   Adapter->call<UrApiKind::urDeviceGetNativeHandle>(getHandleRef(), &Handle);
+  if (getBackend() == backend::opencl) {
+    _OCL_GET_FUNCTION(clRetainDevice)(ur::cast<cl_device_id>(Handle));
+  }
   return Handle;
 }
 

diff --git a/sycl/source/detail/event_impl.cpp b/sycl/source/detail/event_impl.cpp
@@ -500,10 +500,10 @@ ur_native_handle_t event_impl::getNative() {
     this->setHandle(UREvent);
     Handle = UREvent;
   }
-  if (MContext->getBackend() == backend::opencl)
-    Adapter->call<UrApiKind::urEventRetain>(Handle);
   ur_native_handle_t OutHandle;
   Adapter->call<UrApiKind::urEventGetNativeHandle>(Handle, &OutHandle);
+  if (MContext->getBackend() == backend::opencl)
+    _OCL_GET_FUNCTION(clRetainEvent)(ur::cast<cl_event>(OutHandle));
   return OutHandle;
 }
 

diff --git a/sycl/source/detail/kernel_impl.hpp b/sycl/source/detail/kernel_impl.hpp
@@ -75,10 +75,10 @@ class kernel_impl {
   ///
   /// \return a valid cl_kernel instance
   cl_kernel get() const {
-    getAdapter()->call<UrApiKind::urKernelRetain>(MKernel);
     ur_native_handle_t nativeHandle = 0;
     getAdapter()->call<UrApiKind::urKernelGetNativeHandle>(MKernel,
                                                            &nativeHandle);
+    _OCL_GET_FUNCTION(clRetainKernel)(ur::cast<cl_kernel>(nativeHandle));
     return ur::cast<cl_kernel>(nativeHandle);
   }
 
@@ -179,12 +179,13 @@ class kernel_impl {
   ur_native_handle_t getNative() const {
     const AdapterPtr &Adapter = MContext->getAdapter();
 
-    if (MContext->getBackend() == backend::opencl)
-      Adapter->call<UrApiKind::urKernelRetain>(MKernel);
-
     ur_native_handle_t NativeKernel = 0;
     Adapter->call<UrApiKind::urKernelGetNativeHandle>(MKernel, &NativeKernel);
 
+    if (MContext->getBackend() == backend::opencl) {
+      _OCL_GET_FUNCTION(clRetainKernel)(ur::cast<cl_kernel>(NativeKernel));
+    }
+
     return NativeKernel;
   }
 

diff --git a/sycl/source/detail/queue_impl.cpp b/sycl/source/detail/queue_impl.cpp
@@ -731,15 +731,17 @@ void queue_impl::destructorNotification() {
 
 ur_native_handle_t queue_impl::getNative(int32_t &NativeHandleDesc) const {
   const AdapterPtr &Adapter = getAdapter();
-  if (getContextImplPtr()->getBackend() == backend::opencl)
-    Adapter->call<UrApiKind::urQueueRetain>(MQueues[0]);
   ur_native_handle_t Handle{};
   ur_queue_native_desc_t UrNativeDesc{UR_STRUCTURE_TYPE_QUEUE_NATIVE_DESC,
                                       nullptr, nullptr};
   UrNativeDesc.pNativeData = &NativeHandleDesc;
 
   Adapter->call<UrApiKind::urQueueGetNativeHandle>(MQueues[0], &UrNativeDesc,
                                                    &Handle);
+  if (getContextImplPtr()->getBackend() == backend::opencl) {
+    _OCL_GET_FUNCTION(clRetainCommandQueue)
+    (ur::cast<cl_command_queue>(Handle));
+  }
   return Handle;
 }
 

diff --git a/sycl/source/detail/queue_impl.hpp b/sycl/source/detail/queue_impl.hpp
@@ -273,10 +273,11 @@ class queue_impl {
   /// \return an OpenCL interoperability queue handle.
 
   cl_command_queue get() {
-    getAdapter()->call<UrApiKind::urQueueRetain>(MQueues[0]);
     ur_native_handle_t nativeHandle = 0;
     getAdapter()->call<UrApiKind::urQueueGetNativeHandle>(MQueues[0], nullptr,
                                                           &nativeHandle);
+    _OCL_GET_FUNCTION(clRetainCommandQueue)
+    (ur::cast<cl_command_queue>(nativeHandle));
     return ur::cast<cl_command_queue>(nativeHandle);
   }
 

diff --git a/sycl/source/detail/sycl_mem_obj_t.cpp b/sycl/source/detail/sycl_mem_obj_t.cpp
@@ -57,8 +57,9 @@ SYCLMemObjT::SYCLMemObjT(ur_native_handle_t MemObject,
         make_error_code(errc::invalid),
         "Input context must be the same as the context of cl_mem");
 
-  if (MInteropContext->getBackend() == backend::opencl)
-    Adapter->call<UrApiKind::urMemRetain>(MInteropMemObject);
+  if (MInteropContext->getBackend() == backend::opencl) {
+    _OCL_GET_FUNCTION(clRetainMemObject)(ur::cast<cl_mem>(MemObject));
+  }
 }
 
 ur_mem_type_t getImageType(int Dimensions) {
@@ -112,8 +113,9 @@ SYCLMemObjT::SYCLMemObjT(ur_native_handle_t MemObject,
         make_error_code(errc::invalid),
         "Input context must be the same as the context of cl_mem");
 
-  if (MInteropContext->getBackend() == backend::opencl)
-    Adapter->call<UrApiKind::urMemRetain>(MInteropMemObject);
+  if (MInteropContext->getBackend() == backend::opencl) {
+    _OCL_GET_FUNCTION(clRetainMemObject)(ur::cast<cl_mem>(MemObject));
+  }
 }
 
 void SYCLMemObjT::releaseMem(ContextImplPtr Context, void *MemAllocation) {

diff --git a/sycl/source/device.cpp b/sycl/source/device.cpp
@@ -43,7 +43,7 @@ device::device(cl_device_id DeviceId) {
   auto Platform =
       detail::platform_impl::getPlatformFromUrDevice(Device, Adapter);
   impl = Platform->getOrMakeDeviceImpl(Device, Platform);
-  Adapter->call<detail::UrApiKind::urDeviceRetain>(impl->getHandleRef());
+  _OCL_GET_FUNCTION(clRetainDevice)(DeviceId);
 }
 
 device::device(const device_selector &deviceSelector) {

diff --git a/sycl/source/event.cpp b/sycl/source/event.cpp
@@ -30,8 +30,7 @@ event::event(cl_event ClEvent, const context &SyclContext)
   // This is a special interop constructor for OpenCL, so the event must be
   // retained.
   // TODO(pi2ur): Don't just cast from cl_event above
-  impl->getAdapter()->call<detail::UrApiKind::urEventRetain>(
-      detail::ur::cast<ur_event_handle_t>(ClEvent));
+  _OCL_GET_FUNCTION(clRetainEvent)(ClEvent);
 }
 
 bool event::operator==(const event &rhs) const { return rhs.impl == impl; }

diff --git a/sycl/source/kernel.cpp b/sycl/source/kernel.cpp
@@ -30,7 +30,7 @@ kernel::kernel(cl_kernel ClKernel, const context &SyclContext) {
   // This is a special interop constructor for OpenCL, so the kernel must be
   // retained.
   if (get_backend() == backend::opencl) {
-    impl->getAdapter()->call<detail::UrApiKind::urKernelRetain>(hKernel);
+    _OCL_GET_FUNCTION(clRetainKernel)(ClKernel);
   }
 }