[SYCL] Keep `platform_impl`'s `device_impl`s alive until shutdown #18251

aelovikov-intel · 2025-04-29T23:20:22Z

After that devices are never destroyed until the SYCL RT library shutdown. In practice, that means that before the change a simple

int main() { sycl::device d; }

went into platform ctor, then queried all the platform's devices to check that it has some, returned from ctor and those sycl::devices created on stack were already destroyed. After that, when creating user's sycl::device d we were re-creating device hierarchy for the platform at SYCL level again (including some calls to urDeviceGetInfo during device_impl creation).

After the changes, devices created when veryfing that platform isn't empty are preserved inside the platform_impl object and this existing SYCL devices hierarchy is used when creating user's device object.

A note on the implementation: device_impl has an std::shared_ptr<platform_impl> inside so we can't rely on automatic resource management just by the nature of std::shared_ptr everywhere (and we haven't changed this aspect in #18143). As such, we have to perform some explicit resource release during shutdown procedure (or in ~UrMock() for unittests).

After that devices are never destroyed until the SYCL RT library shutdown. In practice, that means that before the change a simple ``` int main() { sycl::device d; } ``` went into `platform` ctor, then queried all the platform's devices to check that it has some, returned from ctor and those `sycl::device`s created on stack were already destroyed. After that, when creating user's `sycl::device d` we were re-creating device hierarchy for the platform at SYCL level again (including some calls to `urDeviceGetInfo` during `device_impl` creation). After the changes, devices created when verying that platform isn't empty are preserved inside the `platform_impl` object and this existing SYCL devices hierarcy is used when creating user's device object.

After intel#18251 device 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 intel#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.

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.

Refactored the `ProgramManager` to use `device_impl &` instead of `const device &`. See #18270 and #18251 that started the refactoring. Signed-off-by: Sergei Vinogradov <[email protected]>

…e_impl *` intel#18251 extended `device_impl`s' lifetimes until shutdown and intel#18270 started to pass devices as raw pointers in some of the APIs. This PR builds on top of that and extends usage of raw pointers/references/`device_range` as the devices are known to be alive and extra `std::shared_ptr`'s atomic increments aren't necessary and could be avoided. This change mostly touches `device_image_impl` and `program_manager` and switches most of the APIs to use `devices_range`. A few number of other modifications are caused by these APIs' changes and are necessary to keep the code buildable.

…e_impl *` (#19459) #18251 extended `device_impl`s' lifetimes until shutdown and #18270 started to pass devices as raw pointers in some of the APIs. This PR builds on top of that and extends usage of raw pointers/references/`device_range` as the devices are known to be alive and extra `std::shared_ptr`'s atomic increments aren't necessary and could be avoided. Since we change the type of `device_image_impl::MDevices`, other APIs in that class and in `program_manager` don't need to operate in terms of `sycl::device` or `std::shared_ptr<device_impl>` and we can switch them to use `devices_range` instead. A small number of other modifications are caused by these APIs' changes and are necessary to keep the code buildable. One extra change is the addition of a minor `devices_range::to<std::vector<ur_device_handle_t>>()` helper that we can use now that most of the arguments are `device_range`. Technically, could go in another PR but then we'd just be modifying the exact same lines two times, so I decided to fuse it here.

…ce_impl *` intel#18251 extended `device_impl`s' lifetimes until shutdown and intel#18270 started to pass devices as raw pointers in some of the APIs. This PR builds on top of that and extends usage of raw pointers/references/`device_range` as the devices are known to be alive and extra `std::shared_ptr`'s atomic increments aren't necessary and could be avoided. Since we change the type of `kernel_bundle_impl::MDevices`, other APIs in that class don't need to operate in terms of `sycl::device` or `std::shared_ptr<device_impl>` and we can switch them to use `devices_range` instead. A small number of other modifications are caused by these APIs' changes and are necessary to keep the code buildable.

…ce_impl *` (#19484) #18251 extended `device_impl`s' lifetimes until shutdown and #18270 started to pass devices as raw pointers in some of the APIs. This PR builds on top of that and extends usage of raw pointers/references/`device_range` as the devices are known to be alive and extra `std::shared_ptr`'s atomic increments aren't necessary and could be avoided. Since we change the type of `kernel_bundle_impl::MDevices`, other APIs in that class don't need to operate in terms of `sycl::device` or `std::shared_ptr<device_impl>` and we can switch them to use `devices_range` instead. A small number of other modifications are caused by these APIs' changes and are necessary to keep the code buildable.

aelovikov-intel requested a review from a team as a code owner April 29, 2025 23:20

aelovikov-intel requested a review from sergey-semenov April 29, 2025 23:20

aelovikov-intel temporarily deployed to WindowsCILock April 29, 2025 23:20 — with GitHub Actions Inactive

aelovikov-intel temporarily deployed to WindowsCILock April 29, 2025 23:36 — with GitHub Actions Inactive

aelovikov-intel temporarily deployed to WindowsCILock April 29, 2025 23:49 — with GitHub Actions Inactive

aelovikov-intel had a problem deploying to WindowsCILock April 29, 2025 23:49 — with GitHub Actions Failure

aelovikov-intel temporarily deployed to WindowsCILock April 30, 2025 06:47 — with GitHub Actions Inactive

sergey-semenov approved these changes Apr 30, 2025

View reviewed changes

aelovikov-intel merged commit 7b8996e into intel:sycl Apr 30, 2025
39 of 43 checks passed

aelovikov-intel deleted the keep-devices-alive branch April 30, 2025 15:45

aelovikov-intel mentioned this pull request Apr 30, 2025

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

Merged

aelovikov-intel mentioned this pull request May 5, 2025

[SYCL] Use refs to device_impl in ProgramManager #18320

Merged

aelovikov-intel mentioned this pull request Jul 15, 2025

[NFCI][SYCL] Change device_image_impl::MDevices to store raw device_impl * #19459

Merged

aelovikov-intel mentioned this pull request Jul 16, 2025

[NFCI][SYCL] Change kernel_bundle_impl::MDevices to store raw device_impl * #19484

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SYCL] Keep `platform_impl`'s `device_impl`s alive until shutdown #18251

[SYCL] Keep `platform_impl`'s `device_impl`s alive until shutdown #18251

Uh oh!

aelovikov-intel commented Apr 29, 2025 •

edited by sergey-semenov

Loading

Uh oh!

Uh oh!

Uh oh!

[SYCL] Keep platform_impl's device_impls alive until shutdown #18251

[SYCL] Keep platform_impl's device_impls alive until shutdown #18251

Uh oh!

Conversation

aelovikov-intel commented Apr 29, 2025 • edited by sergey-semenov Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

[SYCL] Keep `platform_impl`'s `device_impl`s alive until shutdown #18251

[SYCL] Keep `platform_impl`'s `device_impl`s alive until shutdown #18251

aelovikov-intel commented Apr 29, 2025 •

edited by sergey-semenov

Loading