Skip to content

Commit ab4b9b9

Browse files
committed
Do not call urDeviceRetain() in case of subdevices
There is no need to call urDeviceRetain(MDevice) in constructor of device_impl in case of subdevices. It fixes URT-961. Signed-off-by: Lukasz Dorau <[email protected]>
1 parent 74d8de6 commit ab4b9b9

File tree

4 files changed

+53
-4
lines changed

4 files changed

+53
-4
lines changed

sycl/source/detail/device_impl.cpp

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,24 @@ namespace sycl {
1919
inline namespace _V1 {
2020
namespace detail {
2121

22+
/// Constructs a SYCL device instance using the provided
23+
/// UR device instance.
24+
device_impl::device_impl(ur_device_handle_t Device, platform_impl &Platform,
25+
device_impl::private_tag, bool IsSubDevice)
26+
: MDevice(Device), MPlatform(Platform),
27+
// No need to set MRootDevice when MAlwaysRootDevice is true
28+
MRootDevice(Platform.MAlwaysRootDevice
29+
? nullptr
30+
: get_info_impl<UR_DEVICE_INFO_PARENT_DEVICE>()),
31+
// TODO catch an exception and put it to list of asynchronous exceptions:
32+
MCache{*this} {
33+
if (!IsSubDevice) {
34+
// Interoperability Constructor already calls DeviceRetain in
35+
// urDeviceCreateWithNativeHandle.
36+
getAdapter().call<UrApiKind::urDeviceRetain>(MDevice);
37+
}
38+
}
39+
2240
/// Constructs a SYCL device instance using the provided
2341
/// UR device instance.
2442
device_impl::device_impl(ur_device_handle_t Device, platform_impl &Platform,
@@ -30,9 +48,7 @@ device_impl::device_impl(ur_device_handle_t Device, platform_impl &Platform,
3048
: get_info_impl<UR_DEVICE_INFO_PARENT_DEVICE>()),
3149
// TODO catch an exception and put it to list of asynchronous exceptions:
3250
MCache{*this} {
33-
// Interoperability Constructor already calls DeviceRetain in
34-
// urDeviceCreateWithNativeHandle.
35-
getAdapter().call<UrApiKind::urDeviceRetain>(MDevice);
51+
device_impl(Device, Platform, private_tag{}, false);
3652
}
3753

3854
device_impl::~device_impl() {
@@ -164,7 +180,7 @@ std::vector<device> device_impl::create_sub_devices(
164180
std::for_each(SubDevices.begin(), SubDevices.end(),
165181
[&res, this](const ur_device_handle_t &a_ur_device) {
166182
device sycl_device = detail::createSyclObjFromImpl<device>(
167-
MPlatform.getOrMakeDeviceImpl(a_ur_device));
183+
MPlatform.getOrMakeSubDeviceImpl(a_ur_device));
168184
res.push_back(sycl_device);
169185
});
170186
return res;

sycl/source/detail/device_impl.hpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,14 @@ class device_impl : public std::enable_shared_from_this<device_impl> {
434434
explicit device_impl(ur_device_handle_t Device, platform_impl &Platform,
435435
private_tag);
436436

437+
/// Constructs a SYCL device instance using the provided
438+
/// UR device instance.
439+
//
440+
// Must be called through `platform_impl::getOrMakeSubDeviceImpl` only.
441+
// `private_tag` ensures that is true.
442+
explicit device_impl(ur_device_handle_t Device, platform_impl &Platform,
443+
private_tag, bool IsSubDevice);
444+
437445
~device_impl();
438446

439447
/// Get instance of OpenCL device

sycl/source/detail/platform_impl.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,20 @@ device_impl &platform_impl::getOrMakeDeviceImpl(ur_device_handle_t UrDevice) {
311311
return *MDevices.back();
312312
}
313313

314+
device_impl &
315+
platform_impl::getOrMakeSubDeviceImpl(ur_device_handle_t UrSubDevice) {
316+
const std::lock_guard<std::mutex> Guard(MDeviceMapMutex);
317+
// If we've already seen this device, return the impl
318+
if (device_impl *Result = getDeviceImplHelper(UrSubDevice))
319+
return *Result;
320+
321+
// Otherwise make the impl
322+
MDevices.emplace_back(std::make_shared<device_impl>(
323+
UrSubDevice, *this, device_impl::private_tag{}, true /*IsSubDevice*/));
324+
325+
return *MDevices.back();
326+
}
327+
314328
static bool supportsAffinityDomain(const device &dev,
315329
info::partition_property partitionProp,
316330
info::partition_affinity_domain domain) {

sycl/source/detail/platform_impl.hpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,17 @@ class platform_impl : public std::enable_shared_from_this<platform_impl> {
172172
/// \return a device_impl* corresponding to the device
173173
device_impl &getOrMakeDeviceImpl(ur_device_handle_t UrDevice);
174174

175+
/// Queries the device_impl cache to either return a shared_ptr
176+
/// for the device_impl corresponding to the UrSubDevice or add
177+
/// a new entry to the cache
178+
///
179+
/// \param UrSubDevice is the UrSubDevice whose impl is requested
180+
///
181+
/// \param PlatormImpl is the Platform for that Device
182+
///
183+
/// \return a device_impl* corresponding to the subdevice
184+
device_impl &getOrMakeSubDeviceImpl(ur_device_handle_t UrSubDevice);
185+
175186
/// Queries the cache to see if the specified UR platform has been seen
176187
/// before. If so, return the cached platform_impl, otherwise create a new
177188
/// one and cache it.

0 commit comments

Comments
 (0)