[SYCL][NFCI] Ensure device_impl is only created via platform_impl::getOrMakeDeviceImpl (#18227)

Once that is guaranteed, it can enable further refactoring in subsequent
PRs.

Author: aelovikov-intel

sycl/source/detail/device_impl.cpp

@@ -21,7 +21,8 @@ namespace detail {
 
 /// Constructs a SYCL device instance using the provided
 /// UR device instance.
-device_impl::device_impl(ur_device_handle_t Device, platform_impl &Platform)
+device_impl::device_impl(ur_device_handle_t Device, platform_impl &Platform,
+                         device_impl::private_tag)
     : MDevice(Device), MPlatform(Platform.shared_from_this()),
       MDeviceHostBaseTime(std::make_pair(0, 0)) {
   const AdapterPtr &Adapter = Platform.getAdapter();

sycl/source/detail/device_impl.hpp

@@ -33,10 +33,19 @@ class platform_impl;
 
 // TODO: Make code thread-safe
 class device_impl {
+  struct private_tag {
+    explicit private_tag() = default;
+  };
+  friend class platform_impl;
+
 public:
   /// Constructs a SYCL device instance using the provided
   /// UR device instance.
-  explicit device_impl(ur_device_handle_t Device, platform_impl &Platform);
+  //
+  // Must be called through `platform_impl::getOrMakeDeviceImpl` only.
+  // `private_tag` ensures that is true.
+  explicit device_impl(ur_device_handle_t Device, platform_impl &Platform,
+                       private_tag);
 
   ~device_impl();
 

sycl/source/detail/platform_impl.cpp

@@ -304,7 +304,8 @@ platform_impl::getOrMakeDeviceImpl(ur_device_handle_t UrDevice) {
     return Result;
 
   // Otherwise make the impl
-  Result = std::make_shared<device_impl>(UrDevice, *this);
+  Result = std::make_shared<device_impl>(UrDevice, *this,
+                                         device_impl::private_tag{});
   MDeviceCache.emplace_back(Result);
 
   return Result;

sycl/unittests/program_manager/SubDevices.cpp

@@ -106,10 +106,8 @@ TEST(SubDevices, DISABLED_BuildProgramForSubdevices) {
   rootDevice = sycl::detail::getSyclObjImpl(device)->getHandleRef();
   // Initialize sub-devices
   sycl::detail::platform_impl &PltImpl = *sycl::detail::getSyclObjImpl(Plt);
-  auto subDev1 =
-      std::make_shared<sycl::detail::device_impl>(urSubDev1, PltImpl);
-  auto subDev2 =
-      std::make_shared<sycl::detail::device_impl>(urSubDev2, PltImpl);
+  auto subDev1 = PltImpl.getOrMakeDeviceImpl(urSubDev1);
+  auto subDev2 = PltImpl.getOrMakeDeviceImpl(urSubDev2);
   sycl::context Ctx{
       {device, sycl::detail::createSyclObjFromImpl<sycl::device>(subDev1),
        sycl::detail::createSyclObjFromImpl<sycl::device>(subDev2)}};

sycl/unittests/queue/DeviceCheck.cpp

@@ -62,6 +62,18 @@ ur_result_t redefinedDevicePartitionAfter(void *pParams) {
     **params.ppNumDevicesRet = *params.pNumDevices;
   return UR_RESULT_SUCCESS;
 }
+ur_result_t redefinedPlatformGet(void *pParams) {
+  auto params = reinterpret_cast<ur_platform_get_params_t *>(pParams);
+  if (*params->ppNumPlatforms)
+    **params->ppNumPlatforms = 2;
+
+  if (*params->pphPlatforms && *params->pNumEntries > 0) {
+    (*params->pphPlatforms)[0] = reinterpret_cast<ur_platform_handle_t>(1);
+    (*params->pphPlatforms)[1] = reinterpret_cast<ur_platform_handle_t>(2);
+  }
+
+  return UR_RESULT_SUCCESS;
+}
 
 // Check that the device is verified to be either a member of the context or a
 // descendant of its member.
@@ -71,6 +83,8 @@ TEST(QueueDeviceCheck, CheckDeviceRestriction) {
       detail::SYCLConfig<detail::SYCL_ENABLE_DEFAULT_CONTEXTS>::reset);
 
   sycl::unittest::UrMock<> Mock;
+  mock::getCallbacks().set_replace_callback("urPlatformGet",
+                                            &redefinedPlatformGet);
   sycl::platform Plt = sycl::platform();
 
   UrPlatform = detail::getSyclObjImpl(Plt)->getHandleRef();
@@ -116,12 +130,15 @@ TEST(QueueDeviceCheck, CheckDeviceRestriction) {
   // Device is neither of the two.
   {
     ParentDevice = nullptr;
-    device Device = detail::createSyclObjFromImpl<device>(
-        std::make_shared<detail::device_impl>(
-            reinterpret_cast<ur_device_handle_t>(0x01),
-            *detail::getSyclObjImpl(Plt)));
+
+    auto Plts = sycl::platform::get_platforms();
+    EXPECT_TRUE(Plts.size() == 2);
+    sycl::platform OtherPlt = Plts[1];
+
+    device Device = OtherPlt.get_devices()[0];
     queue Q{Device};
-    EXPECT_NE(Q.get_context(), DefaultCtx);
+    auto Ctx = Q.get_context();
+    EXPECT_NE(Ctx, DefaultCtx);
     try {
       queue Q2{DefaultCtx, Device};
       EXPECT_TRUE(false);