Skip to content

Fix stale device global initializers on RTC program reuse #21730

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kweronsx wants to merge 2 commits into
base: sycl
Choose a base branch
from
Open

Fix stale device global initializers on RTC program reuse #21730

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8064243
Fix stale device global initializers on RTC program reuse
kweronsx Mar 30, 2026
8514587
[SYCL][UnitTest] Test device global initializer cleanup on program te…
kweronsx Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions sycl/source/detail/context_impl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,31 @@ void context_impl::addDeviceGlobalInitializer(
}
}

void context_impl::removeDeviceGlobalInitializer(
ur_program_handle_t Program, const RTDeviceBinaryImage *BinImage) {
std::lock_guard<std::mutex> Lock(MDeviceGlobalInitializersMutex);

for (auto It = MDeviceGlobalInitializers.begin();
It != MDeviceGlobalInitializers.end();) {
const bool ProgramMatches = It->first.first == Program;
const bool ImageMatches = !BinImage || It->second.MBinImage == BinImage;

if (!ProgramMatches || !ImageMatches) {
++It;
continue;
}

{
std::lock_guard<std::mutex> InitLock(It->second.MDeviceGlobalInitMutex);
if (!It->second.MDeviceGlobalsFullyInitialized)
--MDeviceGlobalNotInitializedCnt;
It->second.ClearEvents(getAdapter());
}

It = MDeviceGlobalInitializers.erase(It);
}
}

std::vector<ur_event_handle_t> context_impl::initializeDeviceGlobals(
ur_program_handle_t NativePrg, queue_impl &QueueImpl,
detail::kernel_bundle_impl *KernelBundleImplPtr) {
Expand Down
6 changes: 6 additions & 0 deletions sycl/source/detail/context_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,12 @@ class context_impl : public std::enable_shared_from_this<context_impl> {
devices_range Devs,
const RTDeviceBinaryImage *BinImage);

/// Removes device global initializers for a program. If BinImage is not
/// null, only initializers associated with that image are removed.
Copy link
Copy Markdown
Contributor

@sergey-semenov sergey-semenov Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the use case for BinImage == nullptr?

Copy link
Copy Markdown
Contributor Author

@kweronsx kweronsx Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The BinImage == nullptr path acts as a "remove all" overload for the given program handle (when you don't have (or don't need) image-level granularity and just want to purge every initializer entry tied to that program). The current call site in removeImages always passes a specific image, but the nullable default is there for a potential future call site where only the program handle is known (for instance, tearing down a program without tracking which images were ever linked to it).

Copy link
Copy Markdown
Contributor

@sergey-semenov sergey-semenov Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we foresee using that in the near future (i.e. in follow up PRs to this)? Otherwise, I would rather leave that out until we have a use for it, since right now reading this part of the code gives a false impression of how we do cleanup.

void
removeDeviceGlobalInitializer(ur_program_handle_t Program,
const RTDeviceBinaryImage *BinImage = nullptr);

/// Initializes device globals for a program on the associated queue.
std::vector<ur_event_handle_t>
initializeDeviceGlobals(ur_program_handle_t NativePrg, queue_impl &QueueImpl,
Expand Down
1 change: 1 addition & 0 deletions sycl/source/detail/program_manager/program_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1846,6 +1846,7 @@ void ProgramManager::removeImages(sycl_device_binaries DeviceBinary) {
auto CurIt = It++;
if (CurIt->second.second == Img) {
if (auto ContextImpl = CurIt->second.first.lock()) {
ContextImpl->removeDeviceGlobalInitializer(CurIt->first, Img);
ContextImpl->getKernelProgramCache().removeAllRelatedEntries(
Img->getImageID());
}
Expand Down
Loading