Skip to content

Commit 070d79a

Browse files
[SYCL] Make sycl::device use raw device_impl * for its impl
`device_impl`s are owned by the parent platforms and are only destroyed when SYCL RT is unloaded. As such, there is no reason to pay the price of a `std::shared_ptr` a raw non-owning pointers is enough. Use a pointer and not a reference because `sycl::device` is assignable.
1 parent 667c45a commit 070d79a

File tree

19 files changed

+124
-51
lines changed

19 files changed

+124
-51
lines changed

sycl/gdb/libsycl.so-gdb.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,7 @@ def __init__(self, gdb_value):
381381
super().__init__(gdb_value)
382382

383383
def impl_ptr(self):
384-
return self.gdb_value()["impl"]["_M_ptr"]
384+
return self.gdb_value()["impl"]
385385

386386
def backend(self):
387387
char_ptr = SYCLType.char_type().pointer()

sycl/include/sycl/device.hpp

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,14 @@ enum class peer_access {
5757
access_supported = 0x0,
5858
atomics_supported = 0x1,
5959
};
60-
60+
template <typename SYCLObjT> class weak_object;
6161
} // namespace ext::oneapi
6262

6363
/// The SYCL device class encapsulates a single SYCL device on which kernels
6464
/// may be executed.
6565
///
6666
/// \ingroup sycl_api
67-
class __SYCL_STANDALONE_DEBUG __SYCL_EXPORT device
68-
: public detail::OwnerLessBase<device> {
67+
class __SYCL_STANDALONE_DEBUG __SYCL_EXPORT device {
6968
friend sycl::detail::ImplUtils;
7069

7170
public:
@@ -93,6 +92,12 @@ class __SYCL_STANDALONE_DEBUG __SYCL_EXPORT device
9392
/// \param DeviceSelector is SYCL 2020 Device Selector, a simple callable that
9493
/// takes a device and returns an int
9594
template <typename DeviceSelector,
95+
// `device_impl` (used as a parameter in private ctor) is incomplete
96+
// so would result in a error trying to instantiate
97+
// `EnableIfSYCL2020DeviceSelectorInvocable` below. Filter it out
98+
// before trying to do that.
99+
typename = std::enable_if_t<
100+
!std::is_same_v<DeviceSelector, detail::device_impl>>,
96101
typename =
97102
detail::EnableIfSYCL2020DeviceSelectorInvocable<DeviceSelector>>
98103
explicit device(const DeviceSelector &deviceSelector)
@@ -361,14 +366,22 @@ class __SYCL_STANDALONE_DEBUG __SYCL_EXPORT device
361366
/// \return the default context
362367
context ext_oneapi_get_default_context();
363368

369+
// Definitions are in `<sycl/ext/oneapi/weak_object.hpp>` to avoid circular
370+
// dependencies:
371+
inline bool ext_oneapi_owner_before(const device &Other) const noexcept;
372+
inline bool ext_oneapi_owner_before(
373+
const ext::oneapi::weak_object<device> &Other) const noexcept;
374+
364375
// TODO: Remove this diagnostics when __SYCL_WARN_IMAGE_ASPECT is removed.
365376
#if defined(__clang__)
366377
#pragma clang diagnostic pop
367378
#endif // defined(__clang__)
368379

369380
private:
370-
std::shared_ptr<detail::device_impl> impl;
371-
device(std::shared_ptr<detail::device_impl> Impl) : impl(std::move(Impl)) {}
381+
// `device_impl`s are owned by the parent platform, user-visible
382+
// `sycl::device` is non-owning and thus very cheap.
383+
detail::device_impl *impl = nullptr;
384+
device(detail::device_impl &impl) : impl(&impl) {}
372385

373386
ur_native_handle_t getNative() const;
374387

sycl/include/sycl/ext/oneapi/backend/level_zero.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,8 @@ inline device make_device<backend::ext_oneapi_level_zero>(
113113
continue;
114114

115115
for (auto &d : p.get_devices()) {
116-
if (auto maybe_device = find_matching_descendent_device(d, BackendObject))
116+
if (auto maybe_device =
117+
detail::find_matching_descendent_device(d, BackendObject))
117118
return *maybe_device;
118119
}
119120
}

sycl/include/sycl/ext/oneapi/experimental/current_device.hpp

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,8 @@ namespace ext::oneapi::experimental::this_thread {
1616

1717
namespace detail {
1818
using namespace sycl::detail;
19-
// Underlying `std::shared_ptr<device_impl>`'s lifetime is tied to the
20-
// `global_handler`, so a subsequent `lock()` is expected to be successful when
21-
// used from user app. We still go through `std::weak_ptr` here because our own
22-
// unittests are linked statically against SYCL RT objects and have to implement
23-
// some hacks to emulate the lifetime management done by the `global_handler`.
24-
inline std::weak_ptr<device_impl> &get_current_device_impl() {
25-
static thread_local std::weak_ptr<device_impl> current_device{
19+
inline device_impl *&get_current_device_impl() {
20+
static thread_local device_impl *current_device{
2621
getSyclObjImpl(sycl::device{sycl::default_selector_v})};
2722
return current_device;
2823
}
@@ -36,7 +31,7 @@ inline std::weak_ptr<device_impl> &get_current_device_impl() {
3631
/// task or an asynchronous error handler.
3732
inline sycl::device get_current_device() {
3833
return detail::createSyclObjFromImpl<device>(
39-
detail::get_current_device_impl().lock());
34+
*detail::get_current_device_impl());
4035
}
4136

4237
/// @brief Sets the current default device to `dev` for the calling host thread.

sycl/include/sycl/ext/oneapi/weak_object.hpp

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,67 @@ class weak_object<stream> : public detail::weak_object_base<stream> {
233233
weak_object<detail::GlobalBufAccessorT> MWeakGlobalFlushBuf;
234234
};
235235

236+
template <> class weak_object<device> {
237+
friend sycl::device;
238+
239+
detail::device_impl *dev_impl = nullptr;
240+
241+
public:
242+
using object_type = device;
243+
244+
constexpr weak_object() noexcept = default;
245+
weak_object(const device &dev) noexcept
246+
: dev_impl(detail::getSyclObjImpl(dev)) {}
247+
weak_object(const weak_object &Other) noexcept = default;
248+
weak_object(weak_object &&Other) noexcept = default;
249+
250+
weak_object &operator=(const device &Other) noexcept {
251+
this->dev_impl = detail::getSyclObjImpl(Other);
252+
return *this;
253+
}
254+
weak_object &operator=(const weak_object &Other) noexcept = default;
255+
weak_object &operator=(weak_object &&Other) noexcept = default;
256+
257+
bool expired() const noexcept { return dev_impl == nullptr; }
258+
259+
void reset() noexcept { dev_impl = nullptr; }
260+
261+
#ifndef __SYCL_DEVICE_ONLY__
262+
std::optional<device> try_lock() const noexcept {
263+
if (!dev_impl)
264+
return std::nullopt;
265+
return sycl::detail::createSyclObjFromImpl<device>(*dev_impl);
266+
}
267+
device lock() const {
268+
std::optional<device> OptionalObj = try_lock();
269+
if (!OptionalObj)
270+
throw sycl::exception(sycl::make_error_code(sycl::errc::invalid),
271+
"Referenced object has expired.");
272+
return *OptionalObj;
273+
}
274+
bool owner_before(const device &Other) const noexcept {
275+
return dev_impl < detail::getSyclObjImpl(Other);
276+
}
277+
bool owner_before(const weak_object &Other) const noexcept {
278+
return dev_impl < Other.dev_impl;
279+
}
280+
#else
281+
// On device calls to these functions are disallowed, so declare them but
282+
// don't define them to avoid compilation failures.
283+
std::optional<device> try_lock() const noexcept;
284+
device lock() const;
285+
bool owner_before(const device &Other) const noexcept;
286+
bool owner_before(const weak_object &Other) const noexcept;
287+
#endif // __SYCL_DEVICE_ONLY__
288+
};
236289
} // namespace ext::oneapi
290+
inline bool
291+
device::ext_oneapi_owner_before(const device &Other) const noexcept {
292+
return impl < Other.impl;
293+
}
294+
inline bool device::ext_oneapi_owner_before(
295+
const ext::oneapi::weak_object<device> &Other) const noexcept {
296+
return impl < Other.dev_impl;
297+
}
237298
} // namespace _V1
238299
} // namespace sycl

sycl/source/detail/context_impl.hpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,7 @@ class context_impl : public std::enable_shared_from_this<context_impl> {
186186
return false;
187187
}
188188
CurrDevice = detail::getSyclObjImpl(
189-
CurrDevice->get_info<info::device::parent_device>())
190-
.get();
189+
CurrDevice->get_info<info::device::parent_device>());
191190
}
192191

193192
return true;

sycl/source/detail/platform_impl.cpp

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,20 @@ namespace sycl {
3030
inline namespace _V1 {
3131
namespace detail {
3232

33+
platform_impl::platform_impl(ur_platform_handle_t APlatform,
34+
adapter_impl &Adapter)
35+
: MPlatform(APlatform), MAdapter(&Adapter) {
36+
37+
// Find out backend of the platform
38+
ur_backend_t UrBackend = UR_BACKEND_UNKNOWN;
39+
Adapter.call_nocheck<UrApiKind::urPlatformGetInfo>(
40+
APlatform, UR_PLATFORM_INFO_BACKEND, sizeof(ur_backend_t), &UrBackend,
41+
nullptr);
42+
MBackend = convertUrBackend(UrBackend);
43+
}
44+
45+
platform_impl::~platform_impl() = default;
46+
3347
platform_impl &
3448
platform_impl::getOrMakePlatformImpl(ur_platform_handle_t UrPlatform,
3549
adapter_impl &Adapter) {
@@ -266,7 +280,7 @@ device_impl &platform_impl::getOrMakeDeviceImpl(ur_device_handle_t UrDevice) {
266280
return *Result;
267281

268282
// Otherwise make the impl
269-
MDevices.emplace_back(std::make_shared<device_impl>(
283+
MDevices.emplace_back(std::make_unique<device_impl>(
270284
UrDevice, *this, device_impl::private_tag{}));
271285

272286
return *MDevices.back();
@@ -568,7 +582,7 @@ bool platform_impl::has(aspect Aspect) const {
568582
}
569583

570584
device_impl *platform_impl::getDeviceImplHelper(ur_device_handle_t UrDevice) {
571-
for (const std::shared_ptr<device_impl> &Device : MDevices) {
585+
for (const std::unique_ptr<device_impl> &Device : MDevices) {
572586
if (Device->getHandleRef() == UrDevice)
573587
return Device.get();
574588
}

sycl/source/detail/platform_impl.hpp

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,9 @@ class platform_impl : public std::enable_shared_from_this<platform_impl> {
3939
//
4040
// Platforms can only be created under `GlobalHandler`'s ownership via
4141
// `platform_impl::getOrMakePlatformImpl` method.
42-
explicit platform_impl(ur_platform_handle_t APlatform, adapter_impl &Adapter)
43-
: MPlatform(APlatform), MAdapter(&Adapter) {
44-
45-
// Find out backend of the platform
46-
ur_backend_t UrBackend = UR_BACKEND_UNKNOWN;
47-
Adapter.call_nocheck<UrApiKind::urPlatformGetInfo>(
48-
APlatform, UR_PLATFORM_INFO_BACKEND, sizeof(ur_backend_t), &UrBackend,
49-
nullptr);
50-
MBackend = convertUrBackend(UrBackend);
51-
}
42+
explicit platform_impl(ur_platform_handle_t APlatform, adapter_impl &Adapter);
5243

53-
~platform_impl() = default;
44+
~platform_impl();
5445

5546
public:
5647
/// Checks if this platform supports extension.
@@ -221,7 +212,7 @@ class platform_impl : public std::enable_shared_from_this<platform_impl> {
221212

222213
adapter_impl *MAdapter;
223214

224-
std::vector<std::shared_ptr<device_impl>> MDevices;
215+
std::vector<std::unique_ptr<device_impl>> MDevices;
225216
friend class GlobalHandler;
226217
std::mutex MDeviceMapMutex;
227218
};

sycl/source/detail/program_manager/program_manager.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2470,7 +2470,7 @@ device_image_plain ProgramManager::getDeviceImageFromBinaryImage(
24702470
const device &Dev) {
24712471
const bundle_state ImgState = getBinImageState(BinImage);
24722472

2473-
assert(compatibleWithDevice(BinImage, *getSyclObjImpl(Dev).get()));
2473+
assert(compatibleWithDevice(BinImage, *getSyclObjImpl(Dev)));
24742474

24752475
std::shared_ptr<std::vector<sycl::kernel_id>> KernelIDs;
24762476
// Collect kernel names for the image.

sycl/source/detail/usm/usm_impl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ void *alignedAlloc(size_t Alignment, size_t Size, const context &Ctxt,
245245
#endif
246246
void *RetVal =
247247
alignedAllocInternal(Alignment, Size, getSyclObjImpl(Ctxt).get(),
248-
getSyclObjImpl(Dev).get(), Kind, PropList);
248+
getSyclObjImpl(Dev), Kind, PropList);
249249
#ifdef XPTI_ENABLE_INSTRUMENTATION
250250
// Once the allocation is complete, update metadata with the memory pointer
251251
// before the mem_alloc_end event is sent

0 commit comments

Comments
 (0)