Skip to content

[NFC][SYCL] More cleanup after enable_shared_from_this for kernel_bundle_impl #19185

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
wants to merge 2 commits into
base: sycl
Choose a base branch
from
Open

[NFC][SYCL] More cleanup after enable_shared_from_this for kernel_bundle_impl #19185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
549ad94
[NFC][SYCL] More cleanup after `enable_shared_from_this` for `kernel_…
aelovikov-intel Jun 26, 2025
f09d4d0
Merge remote-tracking branch 'origin/sycl' into HEAD
aelovikov-intel Jun 27, 2025
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
6 changes: 3 additions & 3 deletions sycl/source/backend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ kernel make_kernel(const context &TargetContext,
backend Backend) {
const auto &Adapter = getAdapter(Backend);
const auto &ContextImpl = getSyclObjImpl(TargetContext);
const auto &KernelBundleImpl = getSyclObjImpl(KernelBundle);
kernel_bundle_impl &KernelBundleImpl = *getSyclObjImpl(KernelBundle);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to keep the const?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a different const. Before this, it was const std::shared_ptr<T>, not std::shared_ptr<const T>.


// For Level-Zero expect exactly one device image in the bundle. This is
// natural for interop kernel to get created out of a single native
Expand All @@ -334,7 +334,7 @@ kernel make_kernel(const context &TargetContext,
//
ur_program_handle_t UrProgram = nullptr;
if (Backend == backend::ext_oneapi_level_zero) {
if (KernelBundleImpl->size() != 1)
if (KernelBundleImpl.size() != 1)
throw sycl::exception(
sycl::make_error_code(sycl::errc::runtime),
"make_kernel: kernel_bundle must have single program image " +
Expand All @@ -360,7 +360,7 @@ kernel make_kernel(const context &TargetContext,

// Construct the SYCL queue from UR queue.
return detail::createSyclObjFromImpl<kernel>(
std::make_shared<kernel_impl>(UrKernel, *ContextImpl, KernelBundleImpl));
std::make_shared<kernel_impl>(UrKernel, *ContextImpl, &KernelBundleImpl));
}

kernel make_kernel(ur_native_handle_t NativeHandle,
Expand Down
8 changes: 4 additions & 4 deletions sycl/source/detail/device_image_impl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ std::shared_ptr<kernel_impl> device_image_impl::tryGetExtensionKernel(
auto [UrKernel, CacheMutex, ArgMask] =
PM.getOrCreateKernel(Context, AdjustedName,
/*PropList=*/{}, UrProgram);
return std::make_shared<kernel_impl>(
UrKernel, *getSyclObjImpl(Context), shared_from_this(),
OwnerBundle.shared_from_this(), ArgMask, UrProgram, CacheMutex);
return std::make_shared<kernel_impl>(UrKernel, *getSyclObjImpl(Context),
shared_from_this(), OwnerBundle,
ArgMask, UrProgram, CacheMutex);
}
return nullptr;
}
Expand All @@ -49,7 +49,7 @@ std::shared_ptr<kernel_impl> device_image_impl::tryGetExtensionKernel(

return std::make_shared<kernel_impl>(
UrKernel, *detail::getSyclObjImpl(Context), shared_from_this(),
OwnerBundle.shared_from_this(),
OwnerBundle,
/*ArgMask=*/nullptr, UrProgram, /*CacheMutex=*/nullptr);
}

Expand Down
2 changes: 0 additions & 2 deletions sycl/source/detail/handler_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ class dynamic_parameter_impl;
} // namespace ext::oneapi::experimental::detail
namespace detail {

using KernelBundleImplPtr = std::shared_ptr<detail::kernel_bundle_impl>;

enum class HandlerSubmissionState : std::uint8_t {
NO_STATE = 0,
EXPLICIT_KERNEL_BUNDLE_STATE,
Expand Down
15 changes: 7 additions & 8 deletions sycl/source/detail/kernel_bundle_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,10 +412,9 @@ class kernel_bundle_impl
removeDuplicateImages();

for (const kernel_bundle<bundle_state::object> &Bundle : ObjectBundles) {
const KernelBundleImplPtr &BundlePtr = getSyclObjImpl(Bundle);
for (const std::pair<const std::string, std::vector<unsigned char>>
&SpecConst : BundlePtr->MSpecConstValues) {
MSpecConstValues[SpecConst.first] = SpecConst.second;
kernel_bundle_impl &BundleImpl = *getSyclObjImpl(Bundle);
for (const auto &[Name, Values] : BundleImpl.MSpecConstValues) {
MSpecConstValues[Name] = Values;
}
}
}
Expand Down Expand Up @@ -567,7 +566,8 @@ class kernel_bundle_impl

// SYCLBIN constructor
kernel_bundle_impl(const context &Context, const std::vector<device> &Devs,
const sycl::span<char> &Bytes, bundle_state State)
const sycl::span<char> Bytes, bundle_state State,
private_tag)
: MContext(Context), MDevices(Devs), MState(State) {
common_ctor_checks();

Expand Down Expand Up @@ -993,9 +993,8 @@ class kernel_bundle_impl
SelectedImage->get_ur_program_ref());

return std::make_shared<kernel_impl>(
Kernel, *detail::getSyclObjImpl(MContext), SelectedImage,
shared_from_this(), ArgMask, SelectedImage->get_ur_program_ref(),
CacheMutex);
Kernel, *detail::getSyclObjImpl(MContext), SelectedImage, *this,
ArgMask, SelectedImage->get_ur_program_ref(), CacheMutex);
}

std::shared_ptr<kernel_impl>
Expand Down
10 changes: 6 additions & 4 deletions sycl/source/detail/kernel_impl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ inline namespace _V1 {
namespace detail {

kernel_impl::kernel_impl(ur_kernel_handle_t Kernel, context_impl &Context,
KernelBundleImplPtr KernelBundleImpl,
kernel_bundle_impl *KernelBundleImpl,
const KernelArgMask *ArgMask)
: MKernel(Kernel), MContext(Context.shared_from_this()),
MProgram(ProgramManager::getInstance().getUrProgramFromUrKernel(Kernel,
Context)),
MCreatedFromSource(true), MKernelBundleImpl(std::move(KernelBundleImpl)),
MCreatedFromSource(true),
MKernelBundleImpl(KernelBundleImpl ? KernelBundleImpl->shared_from_this()
: nullptr),
MIsInterop(true), MKernelArgMaskPtr{ArgMask} {
ur_context_handle_t UrContext = nullptr;
// Using the adapter from the passed ContextImpl
Expand All @@ -39,14 +41,14 @@ kernel_impl::kernel_impl(ur_kernel_handle_t Kernel, context_impl &Context,

kernel_impl::kernel_impl(ur_kernel_handle_t Kernel, context_impl &ContextImpl,
DeviceImageImplPtr DeviceImageImpl,
KernelBundleImplPtr &&KernelBundleImpl,
const kernel_bundle_impl &KernelBundleImpl,
const KernelArgMask *ArgMask,
ur_program_handle_t Program, std::mutex *CacheMutex)
: MKernel(Kernel), MContext(ContextImpl.shared_from_this()),
MProgram(Program),
MCreatedFromSource(DeviceImageImpl->isNonSYCLSourceBased()),
MDeviceImageImpl(std::move(DeviceImageImpl)),
MKernelBundleImpl(std::move(KernelBundleImpl)),
MKernelBundleImpl(KernelBundleImpl.shared_from_this()),
MIsInterop(MDeviceImageImpl->getOriginMask() & ImageOriginInterop),
MKernelArgMaskPtr{ArgMask}, MCacheMutex{CacheMutex} {
// Enable USM indirect access for interop and non-sycl-jit source kernels.
Expand Down
4 changes: 2 additions & 2 deletions sycl/source/detail/kernel_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class kernel_impl {
/// \param Context is a valid SYCL context
/// \param KernelBundleImpl is a valid instance of kernel_bundle_impl
kernel_impl(ur_kernel_handle_t Kernel, context_impl &Context,
KernelBundleImplPtr KernelBundleImpl,
kernel_bundle_impl *KernelBundleImpl,
const KernelArgMask *ArgMask = nullptr);

/// Constructs a SYCL kernel_impl instance from a SYCL device_image,
Expand All @@ -51,7 +51,7 @@ class kernel_impl {
/// \param KernelBundleImpl is a valid instance of kernel_bundle_impl
kernel_impl(ur_kernel_handle_t Kernel, context_impl &ContextImpl,
DeviceImageImplPtr DeviceImageImpl,
KernelBundleImplPtr &&KernelBundleImpl,
const kernel_bundle_impl &KernelBundleImpl,
const KernelArgMask *ArgMask, ur_program_handle_t Program,
std::mutex *CacheMutex);

Expand Down
2 changes: 1 addition & 1 deletion sycl/source/detail/scheduler/commands.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2540,7 +2540,7 @@ getCGKernelInfo(const CGExecKernel &CommandGroup, context_impl &ContextImpl,
ur_kernel_handle_t UrKernel = nullptr;
std::shared_ptr<device_image_impl> DeviceImageImpl = nullptr;
const KernelArgMask *EliminatedArgMask = nullptr;
auto &KernelBundleImplPtr = CommandGroup.MKernelBundle;
kernel_bundle_impl *KernelBundleImplPtr = CommandGroup.MKernelBundle.get();

if (auto Kernel = CommandGroup.MSyclKernel; Kernel != nullptr) {
UrKernel = Kernel->getHandleRef();
Expand Down
11 changes: 5 additions & 6 deletions sycl/source/kernel_bundle.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ get_kernel_bundle_impl(const context &Ctx, const std::vector<device> &Devs,
detail::KernelBundleImplPtr
get_kernel_bundle_impl(const context &Ctx, const std::vector<device> &Devs,
const sycl::span<char> &Bytes, bundle_state State) {
return std::make_shared<detail::kernel_bundle_impl>(Ctx, Devs, Bytes, State);
return detail::kernel_bundle_impl::create(Ctx, Devs, Bytes, State);
}

detail::KernelBundleImplPtr
Expand Down Expand Up @@ -524,8 +524,8 @@ obj_kb compile_from_source(
LogPtr = &Log;
std::vector<device> UniqueDevices =
sycl::detail::removeDuplicateDevices(Devices);
std::shared_ptr<kernel_bundle_impl> sourceImpl = getSyclObjImpl(SourceKB);
std::shared_ptr<kernel_bundle_impl> KBImpl = sourceImpl->compile_from_source(
kernel_bundle_impl &sourceImpl = *getSyclObjImpl(SourceKB);
std::shared_ptr<kernel_bundle_impl> KBImpl = sourceImpl.compile_from_source(
UniqueDevices, BuildOptions, LogPtr, RegisteredKernelNames);
auto result = sycl::detail::createSyclObjFromImpl<obj_kb>(KBImpl);
if (LogView)
Expand All @@ -548,9 +548,8 @@ exe_kb build_from_source(
LogPtr = &Log;
std::vector<device> UniqueDevices =
sycl::detail::removeDuplicateDevices(Devices);
const std::shared_ptr<kernel_bundle_impl> &sourceImpl =
getSyclObjImpl(SourceKB);
std::shared_ptr<kernel_bundle_impl> KBImpl = sourceImpl->build_from_source(
kernel_bundle_impl &sourceImpl = *getSyclObjImpl(SourceKB);
std::shared_ptr<kernel_bundle_impl> KBImpl = sourceImpl.build_from_source(
UniqueDevices, BuildOptions, LogPtr, RegisteredKernelNames);
auto result = sycl::detail::createSyclObjFromImpl<exe_kb>(std::move(KBImpl));
if (LogView)
Expand Down
Loading