[SYCL] Less shared_ptr for platform_impl (#18143)

`GlobalHandler::MPlatformCache` keeps (shared) ownership of
`platform_impl` objects, so none of them could be destroyed until SYCL
RT library shutdown/unload process. As such, using raw
pointers/reference to `platform_impl` throughout the SYCL RT is totally
fine and would avoid extra costs of `std::shared_ptr`. However, our
unittests don't work the same way and lifetimes are actually managed by
all the data member `std::shared_ptr`s (e.g., see `CurrentDevice.cpp` as
the most obvious case), so simply switching all `PlatformImplPtr` to raw
pointer/reference doesn't work right now. I think it will be possible
with an ABI break if we'd remove all the shared pointers but the one in
the `GlobalHandler`.

What I'm doing here instead is the following:
* Try to keep lifetime management the same, i.e., all data member
pointers are still "smart"
* Inherit from `std::enable_shared_from_this()` so that we could pass
raw references around and only actually create smart pointers when
absolutely necessary.
  * Change internal APIs to operate more using plain references

This should make it much harder to write something like this:
```
  // BAD: Should have been `auto &`, extra atomic counter
  // increment/decrement without it.
  auto PlatformImpl = d.getPlatformImplPtr();
```

Author: aelovikov-intel

sycl/gdb/libsycl.so-gdb.py

@@ -376,7 +376,7 @@ class SYCLDevice(SYCLValue):
 
     IMPL_OFFSET_TO_DEVICE_TYPE = 0x8
     IMPL_OFFSET_TO_PLATFORM = 0x18
-    PLATFORM_OFFSET_TO_BACKEND = 0x10
+    PLATFORM_OFFSET_TO_BACKEND = 0x20
 
     def __init__(self, gdb_value):
         super().__init__(gdb_value)

sycl/include/sycl/detail/impl_utils.hpp

@@ -50,6 +50,14 @@ T createSyclObjFromImpl(
   return T(ImplObj);
 }
 
+template <class T>
+T createSyclObjFromImpl(
+    std::add_lvalue_reference_t<typename std::remove_reference_t<
+        decltype(getSyclObjImpl(std::declval<T>()))>::element_type>
+        ImplRef) {
+  return createSyclObjFromImpl<T>(ImplRef.shared_from_this());
+}
+
 } // namespace detail
 } // namespace _V1
 } // namespace sycl

sycl/source/backend.cpp

@@ -89,9 +89,9 @@ __SYCL_EXPORT device make_device(ur_native_handle_t NativeHandle,
       NativeHandle, Adapter->getUrAdapter(), nullptr, &UrDevice);
 
   // Construct the SYCL device from UR device.
-  auto Platform = platform_impl::getPlatformFromUrDevice(UrDevice, Adapter);
   return detail::createSyclObjFromImpl<device>(
-      Platform->getOrMakeDeviceImpl(UrDevice, Platform));
+      platform_impl::getPlatformFromUrDevice(UrDevice, Adapter)
+          .getOrMakeDeviceImpl(UrDevice));
 }
 
 __SYCL_EXPORT context make_context(ur_native_handle_t NativeHandle,
@@ -288,10 +288,9 @@ make_kernel_bundle(ur_native_handle_t NativeHandle,
   std::transform(
       ProgramDevices.begin(), ProgramDevices.end(), std::back_inserter(Devices),
       [&Adapter](const auto &Dev) {
-        auto Platform =
-            detail::platform_impl::getPlatformFromUrDevice(Dev, Adapter);
-        auto DeviceImpl = Platform->getOrMakeDeviceImpl(Dev, Platform);
-        return createSyclObjFromImpl<device>(DeviceImpl);
+        return createSyclObjFromImpl<device>(
+            detail::platform_impl::getPlatformFromUrDevice(Dev, Adapter)
+                .getOrMakeDeviceImpl(Dev));
       });
 
   // Unlike SYCL, other backends, like OpenCL or Level Zero, may not support

sycl/source/backend/level_zero.cpp

@@ -20,14 +20,13 @@ using namespace sycl::detail;
 __SYCL_EXPORT device make_device(const platform &Platform,
                                  ur_native_handle_t NativeHandle) {
   const auto &Adapter = ur::getAdapter<backend::ext_oneapi_level_zero>();
-  const auto &PlatformImpl = getSyclObjImpl(Platform);
   // Create UR device first.
   ur_device_handle_t UrDevice;
   Adapter->call<UrApiKind::urDeviceCreateWithNativeHandle>(
       NativeHandle, Adapter->getUrAdapter(), nullptr, &UrDevice);
 
   return detail::createSyclObjFromImpl<device>(
-      PlatformImpl->getOrMakeDeviceImpl(UrDevice, PlatformImpl));
+      getSyclObjImpl(Platform)->getOrMakeDeviceImpl(UrDevice));
 }
 
 } // namespace ext::oneapi::level_zero::detail

sycl/source/detail/allowlist.cpp

@@ -375,8 +375,9 @@ void applyAllowList(std::vector<ur_device_handle_t> &UrDevices,
 
   // Get platform's backend and put it to DeviceDesc
   DeviceDescT DeviceDesc;
-  auto PlatformImpl = platform_impl::getOrMakePlatformImpl(UrPlatform, Adapter);
-  backend Backend = PlatformImpl->getBackend();
+  platform_impl &PlatformImpl =
+      platform_impl::getOrMakePlatformImpl(UrPlatform, Adapter);
+  backend Backend = PlatformImpl.getBackend();
 
   for (const auto &SyclBe : getSyclBeMap()) {
     if (SyclBe.second == Backend) {
@@ -395,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, PlatformImpl);
+    auto 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>(

sycl/source/detail/buffer_impl.cpp

@@ -79,12 +79,11 @@ buffer_impl::getNativeVector(backend BackendName) const {
     // doesn't have context and platform
     if (!Ctx)
       continue;
-    const PlatformImplPtr &Platform = Ctx->getPlatformImpl();
-    assert(Platform && "Platform must be present for device context");
-    if (Platform->getBackend() != BackendName)
+    const platform_impl &Platform = Ctx->getPlatformImpl();
+    if (Platform.getBackend() != BackendName)
       continue;
 
-    auto Adapter = Platform->getAdapter();
+    auto Adapter = Platform.getAdapter();
 
     ur_native_handle_t Handle = 0;
     // When doing buffer interop we don't know what device the memory should be
@@ -94,7 +93,7 @@ buffer_impl::getNativeVector(backend BackendName) const {
                                                    &Handle);
     Handles.push_back(Handle);
 
-    if (Platform->getBackend() == backend::opencl) {
+    if (Platform.getBackend() == backend::opencl) {
       __SYCL_OCL_CALL(clRetainMemObject, ur::cast<cl_mem>(Handle));
     }
   }

sycl/source/detail/context_impl.cpp

@@ -41,10 +41,10 @@ context_impl::context_impl(const std::vector<sycl::device> Devices,
                            async_handler AsyncHandler,
                            const property_list &PropList)
     : MOwnedByRuntime(true), MAsyncHandler(AsyncHandler), MDevices(Devices),
-      MContext(nullptr), MPlatform(), MPropList(PropList),
-      MSupportBufferLocationByDevices(NotChecked) {
+      MContext(nullptr),
+      MPlatform(detail::getSyclObjImpl(MDevices[0].get_platform())),
+      MPropList(PropList), MSupportBufferLocationByDevices(NotChecked) {
   verifyProps(PropList);
-  MPlatform = detail::getSyclObjImpl(MDevices[0].get_platform());
   std::vector<ur_device_handle_t> DeviceIds;
   for (const auto &D : MDevices) {
     if (D.has(aspect::ext_oneapi_is_composite)) {
@@ -96,13 +96,13 @@ context_impl::context_impl(ur_context_handle_t UrContext,
           make_error_code(errc::invalid),
           "No devices in the provided device list and native context.");
 
-    std::shared_ptr<detail::platform_impl> Platform =
+    platform_impl &Platform =
         platform_impl::getPlatformFromUrDevice(DeviceIds[0], Adapter);
     for (ur_device_handle_t Dev : DeviceIds) {
-      MDevices.emplace_back(createSyclObjFromImpl<device>(
-          Platform->getOrMakeDeviceImpl(Dev, Platform)));
+      MDevices.emplace_back(
+          createSyclObjFromImpl<device>(Platform.getOrMakeDeviceImpl(Dev)));
     }
-    MPlatform = Platform;
+    MPlatform = Platform.shared_from_this();
   }
   // TODO catch an exception and put it to list of asynchronous exceptions
   // getAdapter() will be the same as the Adapter passed. This should be taken
@@ -158,7 +158,7 @@ uint32_t context_impl::get_info<info::context::reference_count>() const {
                                                           this->getAdapter());
 }
 template <> platform context_impl::get_info<info::context::platform>() const {
-  return createSyclObjFromImpl<platform>(MPlatform);
+  return createSyclObjFromImpl<platform>(*MPlatform);
 }
 template <>
 std::vector<sycl::device>

sycl/source/detail/context_impl.hpp

@@ -29,7 +29,6 @@ inline namespace _V1 {
 // Forward declaration
 class device;
 namespace detail {
-using PlatformImplPtr = std::shared_ptr<detail::platform_impl>;
 class context_impl {
 public:
   /// Constructs a context_impl using a single SYCL devices.
@@ -90,7 +89,7 @@ class context_impl {
   const AdapterPtr &getAdapter() const { return MPlatform->getAdapter(); }
 
   /// \return the PlatformImpl associated with this context.
-  const PlatformImplPtr &getPlatformImpl() const { return MPlatform; }
+  platform_impl &getPlatformImpl() const { return *MPlatform; }
 
   /// Queries this context for information.
   ///
@@ -257,7 +256,8 @@ class context_impl {
   async_handler MAsyncHandler;
   std::vector<device> MDevices;
   ur_context_handle_t MContext;
-  PlatformImplPtr MPlatform;
+  // TODO: Make it a reference instead, but that needs a bit more refactoring:
+  std::shared_ptr<platform_impl> MPlatform;
   property_list MPropList;
   CachedLibProgramsT MCachedLibPrograms;
   std::mutex MCachedLibProgramsMutex;

sycl/source/detail/device_impl.cpp

@@ -21,17 +21,17 @@ namespace detail {
 
 /// Constructs a SYCL device instance using the provided
 /// UR device instance.
-device_impl::device_impl(ur_device_handle_t Device, PlatformImplPtr Platform)
-    : MDevice(Device), MPlatform(Platform),
+device_impl::device_impl(ur_device_handle_t Device, platform_impl &Platform)
+    : MDevice(Device), MPlatform(Platform.shared_from_this()),
       MDeviceHostBaseTime(std::make_pair(0, 0)) {
-  const AdapterPtr &Adapter = Platform->getAdapter();
+  const AdapterPtr &Adapter = Platform.getAdapter();
 
   // TODO catch an exception and put it to list of asynchronous exceptions
   Adapter->call<UrApiKind::urDeviceGetInfo>(
       MDevice, UR_DEVICE_INFO_TYPE, sizeof(ur_device_type_t), &MType, nullptr);
 
   // No need to set MRootDevice when MAlwaysRootDevice is true
-  if (!Platform->MAlwaysRootDevice) {
+  if (!Platform.MAlwaysRootDevice) {
     // TODO catch an exception and put it to list of asynchronous exceptions
     Adapter->call<UrApiKind::urDeviceGetInfo>(
         MDevice, UR_DEVICE_INFO_PARENT_DEVICE, sizeof(ur_device_handle_t),
@@ -177,7 +177,7 @@ std::vector<device> device_impl::create_sub_devices(
   std::for_each(SubDevices.begin(), SubDevices.end(),
                 [&res, this](const ur_device_handle_t &a_ur_device) {
                   device sycl_device = detail::createSyclObjFromImpl<device>(
-                      MPlatform->getOrMakeDeviceImpl(a_ur_device, MPlatform));
+                      MPlatform->getOrMakeDeviceImpl(a_ur_device));
                   res.push_back(sycl_device);
                 });
   return res;

sycl/source/detail/device_impl.hpp

@@ -30,14 +30,13 @@ namespace detail {
 
 // Forward declaration
 class platform_impl;
-using PlatformImplPtr = std::shared_ptr<platform_impl>;
 
 // TODO: Make code thread-safe
 class device_impl {
 public:
   /// Constructs a SYCL device instance using the provided
   /// UR device instance.
-  explicit device_impl(ur_device_handle_t Device, PlatformImplPtr Platform);
+  explicit device_impl(ur_device_handle_t Device, platform_impl &Platform);
 
   ~device_impl();
 
@@ -279,8 +278,7 @@ class device_impl {
   backend getBackend() const { return MPlatform->getBackend(); }
 
   /// @brief  Get the platform impl serving this device
-  /// @return PlatformImplPtr
-  const PlatformImplPtr &getPlatformImpl() const { return MPlatform; }
+  platform_impl &getPlatformImpl() const { return *MPlatform; }
 
   /// Get device info string
   std::string get_device_info_string(ur_device_info_t InfoCode) const;
@@ -292,7 +290,7 @@ class device_impl {
   ur_device_handle_t MDevice = 0;
   ur_device_type_t MType;
   ur_device_handle_t MRootDevice = nullptr;
-  PlatformImplPtr MPlatform;
+  std::shared_ptr<platform_impl> MPlatform;
   bool MUseNativeAssert = false;
   mutable std::string MDeviceName;
   mutable std::once_flag MDeviceNameFlag;