[SYCL][NFCI] Less shared_ptr for device_impl (#18270)

After #18251 devices are guaranteed to
be alive until SYCL RT library shutdown, so we don't have to pass
everything in `std::shared_ptr<device_impl>` and might use raw
pointers/references much more.

That said, constraints from
#18143 (mostly unittests linking
statically and lifetimes of static/thread-local objects following from
that) are still here and I'm addressing them the same way - not totally
changing the ownership model, using `std::enable_shared_from_this` and
keep creating shared pointers for member objects to keep the graph of
resource ownership intact.

Author: aelovikov-intel

sycl/gdb/libsycl.so-gdb.py

@@ -374,8 +374,8 @@ def range_common_array(self):
 class SYCLDevice(SYCLValue):
     """Provides information about a sycl::device from a gdb.Value."""
 
-    IMPL_OFFSET_TO_DEVICE_TYPE = 0x8
-    IMPL_OFFSET_TO_PLATFORM = 0x18
+    IMPL_OFFSET_TO_DEVICE_TYPE = 0x18
+    IMPL_OFFSET_TO_PLATFORM = 0x28
     PLATFORM_OFFSET_TO_BACKEND = 0x20
 
     def __init__(self, gdb_value):

sycl/include/sycl/handler.hpp

@@ -214,7 +214,6 @@ class HandlerAccess;
 class HostTask;
 
 using EventImplPtr = std::shared_ptr<event_impl>;
-using DeviceImplPtr = std::shared_ptr<device_impl>;
 
 template <typename RetType, typename Func, typename Arg>
 static Arg member_ptr_helper(RetType (Func::*)(Arg) const);
@@ -251,7 +250,7 @@ template <typename Type> struct get_kernel_wrapper_name_t {
 };
 
 __SYCL_EXPORT device getDeviceFromHandler(handler &);
-const DeviceImplPtr &getDeviceImplFromHandler(handler &);
+device_impl &getDeviceImplFromHandler(handler &);
 
 // Checks if a device_global has any registered kernel usage.
 __SYCL_EXPORT bool isDeviceGlobalUsedInKernel(const void *DeviceGlobalPtr);
@@ -3303,8 +3302,7 @@ class __SYCL_EXPORT handler {
             typename PropertyListT>
   friend class accessor;
   friend device detail::getDeviceFromHandler(handler &);
-  friend const detail::DeviceImplPtr &
-  detail::getDeviceImplFromHandler(handler &);
+  friend detail::device_impl &detail::getDeviceImplFromHandler(handler &);
 
   template <typename DataT, int Dimensions, access::mode AccessMode,
             access::target AccessTarget, access::placeholder IsPlaceholder>

sycl/source/backend/opencl.cpp

@@ -65,10 +65,9 @@ __SYCL_EXPORT bool has_extension(const sycl::device &SyclDevice,
         "has_extension can only be used with an OpenCL backend");
   }
 
-  std::shared_ptr<sycl::detail::device_impl> DeviceImpl =
-      getSyclObjImpl(SyclDevice);
-  ur_device_handle_t AdapterDevice = DeviceImpl->getHandleRef();
-  const AdapterPtr &Adapter = DeviceImpl->getAdapter();
+  detail::device_impl &DeviceImpl = *getSyclObjImpl(SyclDevice);
+  ur_device_handle_t AdapterDevice = DeviceImpl.getHandleRef();
+  const AdapterPtr &Adapter = DeviceImpl.getAdapter();
 
   // Manual invocation of UR API to avoid using deprecated
   // info::device::extensions call.

sycl/source/detail/allowlist.cpp

@@ -396,7 +396,7 @@ void applyAllowList(std::vector<ur_device_handle_t> &UrDevices,
 
   int InsertIDx = 0;
   for (ur_device_handle_t Device : UrDevices) {
-    auto DeviceImpl = PlatformImpl.getOrMakeDeviceImpl(Device);
+    device_impl &DeviceImpl = PlatformImpl.getOrMakeDeviceImpl(Device);
     // get DeviceType value and put it to DeviceDesc
     ur_device_type_t UrDevType = UR_DEVICE_TYPE_ALL;
     Adapter->call<UrApiKind::urDeviceGetInfo>(
@@ -429,20 +429,18 @@ void applyAllowList(std::vector<ur_device_handle_t> &UrDevices,
     }
     // get DeviceVendorId value and put it to DeviceDesc
     uint32_t DeviceVendorIdUInt =
-        sycl::detail::get_device_info<info::device::vendor_id>(
-            *DeviceImpl.get());
+        sycl::detail::get_device_info<info::device::vendor_id>(DeviceImpl);
     std::stringstream DeviceVendorIdHexStringStream;
     DeviceVendorIdHexStringStream << "0x" << std::hex << DeviceVendorIdUInt;
     const auto &DeviceVendorIdValue = DeviceVendorIdHexStringStream.str();
     DeviceDesc[DeviceVendorIdKeyName] = DeviceVendorIdValue;
     // get DriverVersion value and put it to DeviceDesc
     const std::string &DriverVersionValue =
-        sycl::detail::get_device_info<info::device::driver_version>(
-            *DeviceImpl.get());
+        sycl::detail::get_device_info<info::device::driver_version>(DeviceImpl);
     DeviceDesc[DriverVersionKeyName] = DriverVersionValue;
     // get DeviceName value and put it to DeviceDesc
     const std::string &DeviceNameValue =
-        sycl::detail::get_device_info<info::device::name>(*DeviceImpl.get());
+        sycl::detail::get_device_info<info::device::name>(DeviceImpl);
     DeviceDesc[DeviceNameKeyName] = DeviceNameValue;
 
     // check if we can allow device with such device description DeviceDesc

sycl/source/detail/context_impl.cpp

@@ -284,19 +284,18 @@ KernelProgramCache &context_impl::getKernelProgramCache() const {
   return MKernelProgramCache;
 }
 
-bool context_impl::hasDevice(
-    std::shared_ptr<detail::device_impl> Device) const {
+bool context_impl::hasDevice(const detail::device_impl &Device) const {
   for (auto D : MDevices)
-    if (getSyclObjImpl(D) == Device)
+    if (getSyclObjImpl(D).get() == &Device)
       return true;
   return false;
 }
 
-DeviceImplPtr
+device_impl *
 context_impl::findMatchingDeviceImpl(ur_device_handle_t &DeviceUR) const {
   for (device D : MDevices)
     if (getSyclObjImpl(D)->getHandleRef() == DeviceUR)
-      return getSyclObjImpl(D);
+      return getSyclObjImpl(D).get();
 
   return nullptr;
 }
@@ -356,10 +355,10 @@ std::vector<ur_event_handle_t> context_impl::initializeDeviceGlobals(
     return {};
 
   const AdapterPtr &Adapter = getAdapter();
-  const DeviceImplPtr &DeviceImpl = QueueImpl->getDeviceImplPtr();
+  device_impl &DeviceImpl = QueueImpl->getDeviceImpl();
   std::lock_guard<std::mutex> NativeProgramLock(MDeviceGlobalInitializersMutex);
   auto ImgIt = MDeviceGlobalInitializers.find(
-      std::make_pair(NativePrg, DeviceImpl->getHandleRef()));
+      std::make_pair(NativePrg, DeviceImpl.getHandleRef()));
   if (ImgIt == MDeviceGlobalInitializers.end() ||
       ImgIt->second.MDeviceGlobalsFullyInitialized)
     return {};
@@ -461,12 +460,12 @@ void context_impl::DeviceGlobalInitializer::ClearEvents(
 }
 
 void context_impl::memcpyToHostOnlyDeviceGlobal(
-    const std::shared_ptr<device_impl> &DeviceImpl, const void *DeviceGlobalPtr,
-    const void *Src, size_t DeviceGlobalTSize, bool IsDeviceImageScoped,
-    size_t NumBytes, size_t Offset) {
+    device_impl &DeviceImpl, const void *DeviceGlobalPtr, const void *Src,
+    size_t DeviceGlobalTSize, bool IsDeviceImageScoped, size_t NumBytes,
+    size_t Offset) {
   std::optional<ur_device_handle_t> KeyDevice = std::nullopt;
   if (IsDeviceImageScoped)
-    KeyDevice = DeviceImpl->getHandleRef();
+    KeyDevice = DeviceImpl.getHandleRef();
   auto Key = std::make_pair(DeviceGlobalPtr, KeyDevice);
 
   std::lock_guard<std::mutex> InitLock(MDeviceGlobalUnregisteredDataMutex);
@@ -483,13 +482,12 @@ void context_impl::memcpyToHostOnlyDeviceGlobal(
 }
 
 void context_impl::memcpyFromHostOnlyDeviceGlobal(
-    const std::shared_ptr<device_impl> &DeviceImpl, void *Dest,
-    const void *DeviceGlobalPtr, bool IsDeviceImageScoped, size_t NumBytes,
-    size_t Offset) {
+    device_impl &DeviceImpl, void *Dest, const void *DeviceGlobalPtr,
+    bool IsDeviceImageScoped, size_t NumBytes, size_t Offset) {
 
   std::optional<ur_device_handle_t> KeyDevice = std::nullopt;
   if (IsDeviceImageScoped)
-    KeyDevice = DeviceImpl->getHandleRef();
+    KeyDevice = DeviceImpl.getHandleRef();
   auto Key = std::make_pair(DeviceGlobalPtr, KeyDevice);
 
   std::lock_guard<std::mutex> InitLock(MDeviceGlobalUnregisteredDataMutex);
@@ -580,9 +578,8 @@ context_impl::get_default_memory_pool(const context &Context,
 
   assert(Kind == usm::alloc::device);
 
-  std::shared_ptr<sycl::detail::device_impl> DevImpl =
-      sycl::detail::getSyclObjImpl(Device);
-  ur_device_handle_t DeviceHandle = DevImpl->getHandleRef();
+  detail::device_impl &DevImpl = *detail::getSyclObjImpl(Device);
+  ur_device_handle_t DeviceHandle = DevImpl.getHandleRef();
   const sycl::detail::AdapterPtr &Adapter = this->getAdapter();
 
   // Check dev is already in our list of device pool pairs.

sycl/source/detail/context_impl.hpp

@@ -150,33 +150,35 @@ class context_impl {
   KernelProgramCache &getKernelProgramCache() const;
 
   /// Returns true if and only if context contains the given device.
-  bool hasDevice(std::shared_ptr<detail::device_impl> Device) const;
+  bool hasDevice(const detail::device_impl &Device) const;
 
   /// Returns true if and only if the device can be used within this context.
   /// For OpenCL this is currently equivalent to hasDevice, for other backends
   /// it returns true if the device is either a member of the context or a
   /// descendant of a member.
-  bool isDeviceValid(DeviceImplPtr Device) {
-    while (!hasDevice(Device)) {
-      if (Device->isRootDevice()) {
-        if (Device->has(aspect::ext_oneapi_is_component)) {
+  bool isDeviceValid(detail::device_impl &Device) {
+    detail::device_impl *CurrDevice = &Device;
+    while (!hasDevice(*CurrDevice)) {
+      if (CurrDevice->isRootDevice()) {
+        if (CurrDevice->has(aspect::ext_oneapi_is_component)) {
           // Component devices should be implicitly usable in context created
           // for a composite device they belong to.
-          auto CompositeDevice = Device->get_info<
+          auto CompositeDevice = CurrDevice->get_info<
               ext::oneapi::experimental::info::device::composite_device>();
-          return hasDevice(detail::getSyclObjImpl(CompositeDevice));
+          return hasDevice(*detail::getSyclObjImpl(CompositeDevice));
         }
 
         return false;
-      } else if (Device->getBackend() == backend::opencl) {
+      } else if (CurrDevice->getBackend() == backend::opencl) {
         // OpenCL does not support using descendants of context members within
         // that context yet. We make the exception in case it supports
         // component/composite devices.
         // TODO remove once this limitation is lifted
         return false;
       }
-      Device = detail::getSyclObjImpl(
-          Device->get_info<info::device::parent_device>());
+      CurrDevice = detail::getSyclObjImpl(
+                       CurrDevice->get_info<info::device::parent_device>())
+                       .get();
     }
 
     return true;
@@ -190,7 +192,7 @@ class context_impl {
 
   /// Given a UR device, returns the matching shared_ptr<device_impl>
   /// within this context. May return nullptr if no match discovered.
-  DeviceImplPtr findMatchingDeviceImpl(ur_device_handle_t &DeviceUR) const;
+  device_impl *findMatchingDeviceImpl(ur_device_handle_t &DeviceUR) const;
 
   /// Gets the native handle of the SYCL context.
   ///
@@ -216,16 +218,16 @@ class context_impl {
   initializeDeviceGlobals(ur_program_handle_t NativePrg,
                           const std::shared_ptr<queue_impl> &QueueImpl);
 
-  void memcpyToHostOnlyDeviceGlobal(
-      const std::shared_ptr<device_impl> &DeviceImpl,
-      const void *DeviceGlobalPtr, const void *Src, size_t DeviceGlobalTSize,
-      bool IsDeviceImageScoped, size_t NumBytes, size_t Offset);
+  void memcpyToHostOnlyDeviceGlobal(device_impl &DeviceImpl,
+                                    const void *DeviceGlobalPtr,
+                                    const void *Src, size_t DeviceGlobalTSize,
+                                    bool IsDeviceImageScoped, size_t NumBytes,
+                                    size_t Offset);
 
-  void
-  memcpyFromHostOnlyDeviceGlobal(const std::shared_ptr<device_impl> &DeviceImpl,
-                                 void *Dest, const void *DeviceGlobalPtr,
-                                 bool IsDeviceImageScoped, size_t NumBytes,
-                                 size_t Offset);
+  void memcpyFromHostOnlyDeviceGlobal(device_impl &DeviceImpl, void *Dest,
+                                      const void *DeviceGlobalPtr,
+                                      bool IsDeviceImageScoped, size_t NumBytes,
+                                      size_t Offset);
 
   /// Gets a program associated with a device global from the cache.
   std::optional<ur_program_handle_t>

sycl/source/detail/device_global_map_entry.cpp

@@ -47,20 +47,18 @@ DeviceGlobalUSMMem &DeviceGlobalMapEntry::getOrAllocateDeviceGlobalUSM(
          "USM allocations should not be acquired for device_global with "
          "device_image_scope property.");
   const std::shared_ptr<context_impl> &CtxImpl = QueueImpl->getContextImplPtr();
-  const std::shared_ptr<device_impl> &DevImpl = QueueImpl->getDeviceImplPtr();
+  const device_impl &DevImpl = QueueImpl->getDeviceImpl();
   std::lock_guard<std::mutex> Lock(MDeviceToUSMPtrMapMutex);
 
-  auto DGUSMPtr = MDeviceToUSMPtrMap.find({DevImpl.get(), CtxImpl.get()});
+  auto DGUSMPtr = MDeviceToUSMPtrMap.find({&DevImpl, CtxImpl.get()});
   if (DGUSMPtr != MDeviceToUSMPtrMap.end())
     return DGUSMPtr->second;
 
   void *NewDGUSMPtr = detail::usm::alignedAllocInternal(
-      0, MDeviceGlobalTSize, CtxImpl.get(), DevImpl.get(),
-      sycl::usm::alloc::device);
+      0, MDeviceGlobalTSize, CtxImpl.get(), &DevImpl, sycl::usm::alloc::device);
 
   auto NewAllocIt = MDeviceToUSMPtrMap.emplace(
-      std::piecewise_construct,
-      std::forward_as_tuple(DevImpl.get(), CtxImpl.get()),
+      std::piecewise_construct, std::forward_as_tuple(&DevImpl, CtxImpl.get()),
       std::forward_as_tuple(NewDGUSMPtr));
   assert(NewAllocIt.second &&
          "USM allocation for device and context already happened.");

sycl/source/detail/device_image_impl.hpp

@@ -707,12 +707,12 @@ class device_image_impl {
         getSyclObjImpl(MContext);
 
     for (const auto &SyclDev : Devices) {
-      const DeviceImplPtr &DevImpl = getSyclObjImpl(SyclDev);
+      device_impl &DevImpl = *getSyclObjImpl(SyclDev);
       if (!ContextImpl->hasDevice(DevImpl)) {
         throw sycl::exception(make_error_code(errc::invalid),
                               "device not part of kernel_bundle context");
       }
-      if (!DevImpl->extOneapiCanBuild(MRTCBinInfo->MLanguage)) {
+      if (!DevImpl.extOneapiCanBuild(MRTCBinInfo->MLanguage)) {
         // This error cannot not be exercised in the current implementation, as
         // compatibility with a source language depends on the backend's
         // capabilities and all devices in one context share the same backend in
@@ -799,12 +799,12 @@ class device_image_impl {
         getSyclObjImpl(MContext);
 
     for (const auto &SyclDev : Devices) {
-      DeviceImplPtr DevImpl = getSyclObjImpl(SyclDev);
+      detail::device_impl &DevImpl = *getSyclObjImpl(SyclDev);
       if (!ContextImpl->hasDevice(DevImpl)) {
         throw sycl::exception(make_error_code(errc::invalid),
                               "device not part of kernel_bundle context");
       }
-      if (!DevImpl->extOneapiCanCompile(MRTCBinInfo->MLanguage)) {
+      if (!DevImpl.extOneapiCanCompile(MRTCBinInfo->MLanguage)) {
         // This error cannot not be exercised in the current implementation, as
         // compatibility with a source language depends on the backend's
         // capabilities and all devices in one context share the same backend in

sycl/source/detail/device_impl.cpp

@@ -893,10 +893,10 @@ bool device_impl::extOneapiCanBuild(
     ext::oneapi::experimental::source_language Language) {
   try {
     // Get the shared_ptr to this object from the platform that owns it.
-    std::shared_ptr<device_impl> Self = MPlatform->getOrMakeDeviceImpl(MDevice);
+    device_impl &Self = MPlatform->getOrMakeDeviceImpl(MDevice);
     return sycl::ext::oneapi::experimental::detail::
         is_source_kernel_bundle_supported(Language,
-                                          std::vector<DeviceImplPtr>{Self});
+                                          std::vector<device_impl *>{&Self});
 
   } catch (sycl::exception &) {
     return false;
@@ -907,11 +907,11 @@ bool device_impl::extOneapiCanCompile(
     ext::oneapi::experimental::source_language Language) {
   try {
     // Currently only SYCL language is supported for compiling.
-    std::shared_ptr<device_impl> Self = MPlatform->getOrMakeDeviceImpl(MDevice);
+    device_impl &Self = MPlatform->getOrMakeDeviceImpl(MDevice);
     return Language == ext::oneapi::experimental::source_language::sycl &&
            sycl::ext::oneapi::experimental::detail::
                is_source_kernel_bundle_supported(
-                   Language, std::vector<DeviceImplPtr>{Self});
+                   Language, std::vector<device_impl *>{&Self});
   } catch (sycl::exception &) {
     return false;
   }

sycl/source/detail/device_impl.hpp

@@ -32,7 +32,7 @@ namespace detail {
 class platform_impl;
 
 // TODO: Make code thread-safe
-class device_impl {
+class device_impl : public std::enable_shared_from_this<device_impl> {
   struct private_tag {
     explicit private_tag() = default;
   };