[SYCL] Complete transition to `Managed<ur_program_handle_t>` RAII model #19557

aelovikov-intel · 2025-07-22T22:32:44Z

Started in #19536.

Non-NFC because fixes a few resource leaks as can be seen in the updated test.

aelovikov-intel · 2025-07-23T14:46:29Z

sycl/source/detail/program_manager/program_manager.cpp

-  // Should be `std::move(Program)` once `LinkPrograms` is switched to
-  // `Managed<ur_program_handle_t`:


Not relevant, context_impl owns them:

llvm/sycl/source/detail/context_impl.hpp

Lines 135 to 137 in 8861f6b

using CachedLibProgramsT =

std::map<std::pair<DeviceLibExt, ur_device_handle_t>,

Managed<ur_program_handle_t>>;

so LinkPrograms here are non-managed.

uditagarwal97

LGTM overall.
I like that now we don't have to manually call urProgramRetain/Release.

…uction_ocl.cpp` In preparation for intel#19557 and subsequent leak fix.

…ruction_ocl.cpp` (#19565) In preparation for #19557 and subsequent leak fix.

Similar to what's been done with `ur_program_handle_t` in intel#19536 intel#19557

Similar to what's been done with `ur_program_handle_t` in #19536 #19557

aelovikov-intel had a problem deploying to WindowsCILock July 22, 2025 22:32 — with GitHub Actions Error

aelovikov-intel added 25 commits July 22, 2025 15:43

program_manager.cpp: static create*

993aed3

ProgramManager::createURProgram

e18c0c1

program_manager.cpp: loadDeviceLib

5d209d3

ProgramManager::getOrCreateURProgram

11ffadc

BuildF lambda, hopefully...

0ea85a3

FastKernelCacheVal ctor

7d7531a

extKernelCompilerFetchFromCache

6e3345d

createProgramFromSource

9962068

device_image_impl ctor overload 0

e9feebf

device_image_impl ctor overload 1

af403f0

Drop unused device_image_impl ctor overload 2

dd34ba5

Last affected device_image_impl ctor overload

da04557

Move up

5c57574

Debug prints

1799d4f

"Outer" getBuiltURProgram overload

fe03fa8

Remaining ProgramManager::getBuiltURProgram overload

e2dbbcc

backend.cpp

c97ec87

ProgramBuildResult ctor simplification

854a46a

debug print old

7432a51

Proper retain in insertBuiltProgram

f0229bf

One more retain

944ac98

Remove unused Adapter after managed retains

11e0a36

loadDeviceLibFallback (last retain)

f5221d9

Drop debug printing

4f06a1a

clang-format

f59f3c6

aelovikov-intel force-pushed the managed branch from d898435 to f59f3c6 Compare July 22, 2025 22:44

aelovikov-intel marked this pull request as ready for review July 22, 2025 22:44

aelovikov-intel requested a review from a team as a code owner July 22, 2025 22:44

aelovikov-intel requested a review from uditagarwal97 July 22, 2025 22:44

aelovikov-intel had a problem deploying to WindowsCILock July 23, 2025 14:37 — with GitHub Actions Error

Remove stale comment - context_impl owns these

fff3362

aelovikov-intel temporarily deployed to WindowsCILock July 23, 2025 14:43 — with GitHub Actions Inactive

aelovikov-intel commented Jul 23, 2025

View reviewed changes

uditagarwal97 approved these changes Jul 23, 2025

View reviewed changes

aelovikov-intel temporarily deployed to WindowsCILock July 23, 2025 15:09 — with GitHub Actions Inactive

aelovikov-intel marked this pull request as draft July 23, 2025 16:46

aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 23, 2025

[NFC][SYCL][E2E] Trace urProgram* resource in `Basic/interop/constr…

ac78317

…uction_ocl.cpp` In preparation for intel#19557 and subsequent leak fix.

aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 23, 2025

[NFC][SYCL][E2E] Trace urProgram* resource in `Basic/interop/constr…

e86f159

…uction_ocl.cpp` In preparation for intel#19557 and subsequent leak fix.

aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 23, 2025

[NFC][SYCL][E2E] Trace urProgram* resource in `Basic/interop/constr…

058b111

…uction_ocl.cpp` In preparation for intel#19557 and subsequent leak fix.

aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 23, 2025

[NFC][SYCL][E2E] Trace urProgram* resource in `Basic/interop/constr…

c246395

…uction_ocl.cpp` In preparation for intel#19557 and subsequent leak fix.

aelovikov-intel mentioned this pull request Jul 23, 2025

[NFC][SYCL][E2E] Trace urProgram* resources in Basic/interop/construction_ocl.cpp #19565

Merged

aelovikov-intel added a commit that referenced this pull request Jul 23, 2025

[NFC][SYCL][E2E] Trace urProgram* resources in `Basic/interop/const…

7b5838c

…ruction_ocl.cpp` (#19565) In preparation for #19557 and subsequent leak fix.

aelovikov-intel added 2 commits July 23, 2025 13:33

Fix one leak

ebf3ce1

Merge remote-tracking branch 'origin/sycl' into HEAD

134bf45

aelovikov-intel changed the title ~~[NFC][SYCL] Complete transition to Managed<ur_program_handle_t> RAII model~~ [SYCL] Complete transition to Managed<ur_program_handle_t> RAII model Jul 23, 2025

Update test

8fcca38

aelovikov-intel marked this pull request as ready for review July 23, 2025 20:36

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

aelovikov-intel temporarily deployed to WindowsCILock July 23, 2025 21:01 — with GitHub Actions Inactive

uditagarwal97 approved these changes Jul 23, 2025

View reviewed changes

aelovikov-intel merged commit 6653a66 into intel:sycl Jul 23, 2025
25 checks passed

aelovikov-intel deleted the managed branch July 23, 2025 22:12

aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 23, 2025

[NFCI][SYCL] Switch to using Managed<ur_kernel_handle_t>

c12f881

Similar to what's been done with `ur_program_handle_t` in intel#19536 intel#19557

aelovikov-intel mentioned this pull request Jul 23, 2025

[NFCI][SYCL] Switch to using Managed<ur_kernel_handle_t> #19570

Merged

aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 24, 2025

[NFCI][SYCL] Switch to using Managed<ur_kernel_handle_t>

f515231

Similar to what's been done with `ur_program_handle_t` in intel#19536 intel#19557

aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 24, 2025

[NFCI][SYCL] Switch to using Managed<ur_kernel_handle_t>

3bd075b

Similar to what's been done with `ur_program_handle_t` in intel#19536 intel#19557

aelovikov-intel added a commit that referenced this pull request Jul 25, 2025

[NFCI][SYCL] Switch to using Managed<ur_kernel_handle_t> (#19570)

0dc91aa

Similar to what's been done with `ur_program_handle_t` in #19536 #19557

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SYCL] Complete transition to `Managed<ur_program_handle_t>` RAII model #19557

[SYCL] Complete transition to `Managed<ur_program_handle_t>` RAII model #19557

Uh oh!

aelovikov-intel commented Jul 22, 2025 •

edited

Loading

Uh oh!

aelovikov-intel Jul 23, 2025

Uh oh!

uditagarwal97 left a comment

Uh oh!

Uh oh!

Uh oh!

		// Should be `std::move(Program)` once `LinkPrograms` is switched to
		// `Managed<ur_program_handle_t`:

	using CachedLibProgramsT =
	std::map<std::pair<DeviceLibExt, ur_device_handle_t>,
	Managed<ur_program_handle_t>>;

[SYCL] Complete transition to Managed<ur_program_handle_t> RAII model #19557

[SYCL] Complete transition to Managed<ur_program_handle_t> RAII model #19557

Uh oh!

Conversation

aelovikov-intel commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aelovikov-intel Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

uditagarwal97 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

[SYCL] Complete transition to `Managed<ur_program_handle_t>` RAII model #19557

[SYCL] Complete transition to `Managed<ur_program_handle_t>` RAII model #19557

aelovikov-intel commented Jul 22, 2025 •

edited

Loading