[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.

sycl/cmake/modules/AddSYCLUnitTest.cmake

@@ -7,6 +7,8 @@ macro(add_sycl_unittest test_dirname link_variant)
   set(LLVM_REQUIRES_EH ON)
   set(LLVM_REQUIRES_RTTI ON)
 
+  get_target_property(SYCL_BINARY_DIR sycl-toolchain BINARY_DIR)
+
   string(TOLOWER "${CMAKE_BUILD_TYPE}" build_type_lower)
   if (MSVC AND build_type_lower MATCHES "debug")
     set(sycl_obj_target "sycld_object")
@@ -47,7 +49,7 @@ macro(add_sycl_unittest test_dirname link_variant)
         SYCL_CONFIG_FILE_NAME=null.cfg
         SYCL_DEVICELIB_NO_FALLBACK=1
         SYCL_CACHE_DIR="${CMAKE_BINARY_DIR}/sycl_cache"
-        "PATH=${CMAKE_BINARY_DIR}/bin;$ENV{PATH}"
+        "PATH=${SYCL_BINARY_DIR}/unittests/lib;${CMAKE_BINARY_DIR}/bin;$ENV{PATH}"
         ${CMAKE_CURRENT_BINARY_DIR}/${test_dirname}
         DEPENDS
         ${test_dirname}
@@ -59,7 +61,7 @@ macro(add_sycl_unittest test_dirname link_variant)
         SYCL_CONFIG_FILE_NAME=null.cfg
         SYCL_DEVICELIB_NO_FALLBACK=1
         SYCL_CACHE_DIR="${CMAKE_BINARY_DIR}/sycl_cache"
-        "LD_LIBRARY_PATH=${CMAKE_BINARY_DIR}/lib:$ENV{LD_LIBRARY_PATH}"
+        "LD_LIBRARY_PATH=${SYCL_BINARY_DIR}/unittests/lib:${CMAKE_BINARY_DIR}/lib:$ENV{LD_LIBRARY_PATH}"
         ${CMAKE_CURRENT_BINARY_DIR}/${test_dirname}
         DEPENDS
         ${test_dirname}
@@ -73,10 +75,11 @@ macro(add_sycl_unittest test_dirname link_variant)
       LLVMTestingSupport
       OpenCL-Headers
       unified-runtime::mock
+      mockOpenCL
       ${SYCL_LINK_LIBS}
     )
 
-  add_dependencies(${test_dirname} ur_adapter_mock)
+  add_dependencies(${test_dirname} ur_adapter_mock mockOpenCL)
 
   if(SYCL_ENABLE_EXTENSION_JIT)
     target_link_libraries(${test_dirname} PRIVATE sycl-jit)

sycl/include/sycl/detail/common.hpp

@@ -8,6 +8,11 @@
 
 #pragma once
 
+#if _MSC_VER
+#include <windows.h>
+#else
+#include <dlfcn.h>
+#endif
 #include <sycl/detail/defines_elementary.hpp> // for __SYCL_ALWAYS_INLINE
 #include <sycl/detail/export.hpp>             // for __SYCL_EXPORT
 
@@ -379,6 +384,43 @@ 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
+// `WinName`. In Linux, it uses `LinName`.
+//
+// The library must already have been loaded (perhaps by UR), otherwise this
+// function throws.
+template <typename fn>
+fn *dynLookupFunction([[maybe_unused]] const char *WinName,
+                      [[maybe_unused]] const char *LinName,
+                      const char *FunName) {
+#ifdef _MSC_VER
+  auto handle = GetModuleHandleA(WinName);
+  if (!handle) {
+    throw sycl::exception(make_error_code(errc::runtime),
+                          "OpenCL library is not loaded");
+  }
+  auto *retVal = GetProcAddress(handle, FunName);
+#else
+  auto handle = dlopen(LinName, RTLD_LAZY | RTLD_NOLOAD);
+  if (!handle) {
+    throw sycl::exception(make_error_code(errc::runtime),
+                          "OpenCL library is not loaded");
+  }
+  auto *retVal = dlsym(handle, FunName);
+  dlclose(handle);
+#endif
+  if (!handle) {
+    throw sycl::exception(make_error_code(errc::runtime),
+                          "OpenCL method could not be found");
+  }
+  return reinterpret_cast<fn *>(retVal);
+}
+#define _OCL_GET_FUNCTION(FN)                                                  \
+  (::sycl::_V1::detail::dynLookupFunction<decltype(FN)>("OpenCL",              \
+                                                        "libOpenCL.so", #FN))
+
 } // namespace detail
 } // namespace _V1
 } // namespace sycl

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>(

sycl/source/detail/buffer_impl.cpp

@@ -87,7 +87,7 @@ buffer_impl::getNativeVector(backend BackendName) const {
     auto Adapter = Platform->getAdapter();
 
     if (Platform->getBackend() == backend::opencl) {
-      Adapter->call<UrApiKind::urMemRetain>(NativeMem);
+      _OCL_GET_FUNCTION(clRetainMemObject)(ur::cast<cl_mem>(NativeMem));
     }
 
     ur_native_handle_t Handle = 0;

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;
 }
 

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;
   }

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;
 }
 

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;
 }
 

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;
   }
 

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) {
+    auto *RetainFn = _OCL_GET_FUNCTION(clRetainCommandQueue);
+    RetainFn(ur::cast<cl_command_queue>(Handle));
+  }
   return Handle;
 }
 

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);
+    auto *retainFn = _OCL_GET_FUNCTION(clRetainCommandQueue);
+    retainFn(ur::cast<cl_command_queue>(nativeHandle));
     return ur::cast<cl_command_queue>(nativeHandle);
   }
 

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) {

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) {

sycl/source/event.cpp

@@ -29,9 +29,7 @@ event::event(cl_event ClEvent, const context &SyclContext)
           detail::ur::cast<ur_event_handle_t>(ClEvent), 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; }

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);
   }
 }
 

sycl/test/Unit/lit.cfg.py

@@ -70,8 +70,11 @@ def find_shlibpath_var():
 for shlibpath_var in find_shlibpath_var():
     # in stand-alone builds, shlibdir is clang's build tree
     # while llvm_libs_dir is installed LLVM (and possibly older clang)
+    # For unit tests, we have a "mock" OpenCL which needs to have
+    # priority and so is at the start of the shlibpath list
     shlibpath = os.path.pathsep.join(
         (
+            os.path.join(config.test_exec_root, "lib"),
             config.shlibdir,
             config.llvm_libs_dir,
             config.environment.get(shlibpath_var, ""),

sycl/unittests/CMakeLists.txt

@@ -23,6 +23,8 @@ string(TOLOWER "${CMAKE_BUILD_TYPE}" build_type_lower)
 
 include(AddSYCLUnitTest)
 
+add_subdirectory(mock_opencl)
+
 add_custom_target(check-sycl-unittests)
 
 add_subdirectory(ur)

sycl/unittests/Extensions/CompositeDevice.cpp

@@ -1,4 +1,5 @@
 #include "sycl/platform.hpp"
+#include <detail/device_impl.hpp>
 #include <sycl/sycl.hpp>
 
 #include <helpers/UrMock.hpp>
@@ -143,8 +144,7 @@ TEST(CompositeDeviceTest, PlatformExtOneAPIGetCompositeDevices) {
   // We don't expect to see COMPOSITE_DEVICE_1 here, because one of its
   // components (COMPONENT_DEVICE_D) is not available.
   ASSERT_EQ(Composites.size(), 1u);
-  ASSERT_EQ(sycl::bit_cast<ur_device_handle_t>(
-                sycl::get_native<sycl::backend::opencl>(Composites.front())),
+  ASSERT_EQ(sycl::detail::getSyclObjImpl(Composites.front())->getHandleRef(),
             COMPOSITE_DEVICE_0);
 }
 
@@ -162,8 +162,7 @@ TEST(CompositeDeviceTest, SYCLExtOneAPIExperimentalGetCompositeDevices) {
   // We don't expect to see COMPOSITE_DEVICE_1 here, because one of its
   // components (COMPONENT_DEVICE_D) is not available.
   ASSERT_EQ(Composites.size(), 1u);
-  ASSERT_EQ(sycl::bit_cast<ur_device_handle_t>(
-                sycl::get_native<sycl::backend::opencl>(Composites.front())),
+  ASSERT_EQ(sycl::detail::getSyclObjImpl(Composites.front())->getHandleRef(),
             COMPOSITE_DEVICE_0);
 }
 

sycl/unittests/SYCL2020/GetNativeOpenCL.cpp

@@ -13,6 +13,7 @@
 #include <sycl/backend/opencl.hpp>
 #include <sycl/sycl.hpp>
 
+#include <helpers/KernelInteropCommon.hpp>
 #include <helpers/TestKernel.hpp>
 #include <helpers/UrMock.hpp>
 
@@ -115,14 +116,26 @@ TEST(GetNative, GetNativeHandle) {
     cgh.single_task<TestKernel<KS>>([=]() { (void)Acc; });
   });
 
+  ASSERT_EQ(mockOpenCLNumContextRetains(), 0ul);
+  ASSERT_EQ(mockOpenCLNumQueueRetains(), 0ul);
+  ASSERT_EQ(mockOpenCLNumDeviceRetains(), 0ul);
+  ASSERT_EQ(mockOpenCLNumEventRetains(), 0ul);
+  ASSERT_EQ(TestCounter, 2 + DeviceRetainCounter - 1)
+      << "Not all the retain methods were called";
+
   get_native<backend::opencl>(Context);
   get_native<backend::opencl>(Queue);
   get_native<backend::opencl>(Device);
   get_native<backend::opencl>(Event);
   get_native<backend::opencl>(Buffer);
 
-  // Depending on global caches state, urDeviceRetain is called either once or
-  // twice, so there'll be 6 or 7 calls.
-  ASSERT_EQ(TestCounter, 6 + DeviceRetainCounter - 1)
-      << "Not all the retain methods were called";
+  ASSERT_EQ(mockOpenCLNumContextRetains(), 1ul);
+  ASSERT_EQ(mockOpenCLNumQueueRetains(), 1ul);
+  ASSERT_EQ(mockOpenCLNumDeviceRetains(), 1ul);
+  ASSERT_EQ(mockOpenCLNumEventRetains(), 1ul);
+
+  // get_native shouldn't retain the SYCL objects, but instead retains the
+  // underlying handles
+  ASSERT_EQ(TestCounter, 2 + DeviceRetainCounter - 1)
+      << "get_native retained SYCL objects";
 }