Skip to content

Commit 359913e

Browse files
aelovikov-intelaelovikov-intel
authored
[NFC][SYCL] More cleanup after enable_shared_from_this for kernel_bundle_impl (#19185)
Follow-up for #18899. Also adds proper `private_tag` argument for the ctor added in #18949 that missed that. Also pass `span` by value while on it.
1 parent 0a27e3f commit 359913e

File tree

8 files changed

+28
-30
lines changed

8 files changed

+28
-30
lines changed

‎sycl/source/backend.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ kernel make_kernel(const context &TargetContext,
323323
backend Backend) {
324324
const auto &Adapter = getAdapter(Backend);
325325
const auto &ContextImpl = getSyclObjImpl(TargetContext);
326-
const auto &KernelBundleImpl = getSyclObjImpl(KernelBundle);
326+
kernel_bundle_impl &KernelBundleImpl = *getSyclObjImpl(KernelBundle);
327327

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

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

366366
kernel make_kernel(ur_native_handle_t NativeHandle,

‎sycl/source/detail/device_image_impl.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ std::shared_ptr<kernel_impl> device_image_impl::tryGetExtensionKernel(
3333
auto [UrKernel, CacheMutex, ArgMask] =
3434
PM.getOrCreateKernel(Context, AdjustedName,
3535
/*PropList=*/{}, UrProgram);
36-
return std::make_shared<kernel_impl>(
37-
UrKernel, *getSyclObjImpl(Context), shared_from_this(),
38-
OwnerBundle.shared_from_this(), ArgMask, UrProgram, CacheMutex);
36+
return std::make_shared<kernel_impl>(UrKernel, *getSyclObjImpl(Context),
37+
shared_from_this(), OwnerBundle,
38+
ArgMask, UrProgram, CacheMutex);
3939
}
4040
return nullptr;
4141
}
@@ -49,7 +49,7 @@ std::shared_ptr<kernel_impl> device_image_impl::tryGetExtensionKernel(
4949

5050
return std::make_shared<kernel_impl>(
5151
UrKernel, *detail::getSyclObjImpl(Context), shared_from_this(),
52-
OwnerBundle.shared_from_this(),
52+
OwnerBundle,
5353
/*ArgMask=*/nullptr, UrProgram, /*CacheMutex=*/nullptr);
5454
}
5555

‎sycl/source/detail/handler_impl.hpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ class dynamic_parameter_impl;
2222
} // namespace ext::oneapi::experimental::detail
2323
namespace detail {
2424

25-
using KernelBundleImplPtr = std::shared_ptr<detail::kernel_bundle_impl>;
26-
2725
enum class HandlerSubmissionState : std::uint8_t {
2826
NO_STATE = 0,
2927
EXPLICIT_KERNEL_BUNDLE_STATE,

‎sycl/source/detail/kernel_bundle_impl.hpp

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -412,10 +412,9 @@ class kernel_bundle_impl
412412
removeDuplicateImages();
413413

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

568567
// SYCLBIN constructor
569568
kernel_bundle_impl(const context &Context, const std::vector<device> &Devs,
570-
const sycl::span<char> &Bytes, bundle_state State)
569+
const sycl::span<char> Bytes, bundle_state State,
570+
private_tag)
571571
: MContext(Context), MDevices(Devs), MState(State) {
572572
common_ctor_checks();
573573

@@ -993,9 +993,8 @@ class kernel_bundle_impl
993993
SelectedImage->get_ur_program_ref());
994994

995995
return std::make_shared<kernel_impl>(
996-
Kernel, *detail::getSyclObjImpl(MContext), SelectedImage,
997-
shared_from_this(), ArgMask, SelectedImage->get_ur_program_ref(),
998-
CacheMutex);
996+
Kernel, *detail::getSyclObjImpl(MContext), SelectedImage, *this,
997+
ArgMask, SelectedImage->get_ur_program_ref(), CacheMutex);
999998
}
1000999

10011000
std::shared_ptr<kernel_impl>

‎sycl/source/detail/kernel_impl.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,14 @@ inline namespace _V1 {
1717
namespace detail {
1818

1919
kernel_impl::kernel_impl(ur_kernel_handle_t Kernel, context_impl &Context,
20-
KernelBundleImplPtr KernelBundleImpl,
20+
kernel_bundle_impl *KernelBundleImpl,
2121
const KernelArgMask *ArgMask)
2222
: MKernel(Kernel), MContext(Context.shared_from_this()),
2323
MProgram(ProgramManager::getInstance().getUrProgramFromUrKernel(Kernel,
2424
Context)),
25-
MCreatedFromSource(true), MKernelBundleImpl(std::move(KernelBundleImpl)),
25+
MCreatedFromSource(true),
26+
MKernelBundleImpl(KernelBundleImpl ? KernelBundleImpl->shared_from_this()
27+
: nullptr),
2628
MIsInterop(true), MKernelArgMaskPtr{ArgMask} {
2729
ur_context_handle_t UrContext = nullptr;
2830
// Using the adapter from the passed ContextImpl
@@ -39,14 +41,14 @@ kernel_impl::kernel_impl(ur_kernel_handle_t Kernel, context_impl &Context,
3941

4042
kernel_impl::kernel_impl(ur_kernel_handle_t Kernel, context_impl &ContextImpl,
4143
DeviceImageImplPtr DeviceImageImpl,
42-
KernelBundleImplPtr &&KernelBundleImpl,
44+
const kernel_bundle_impl &KernelBundleImpl,
4345
const KernelArgMask *ArgMask,
4446
ur_program_handle_t Program, std::mutex *CacheMutex)
4547
: MKernel(Kernel), MContext(ContextImpl.shared_from_this()),
4648
MProgram(Program),
4749
MCreatedFromSource(DeviceImageImpl->isNonSYCLSourceBased()),
4850
MDeviceImageImpl(std::move(DeviceImageImpl)),
49-
MKernelBundleImpl(std::move(KernelBundleImpl)),
51+
MKernelBundleImpl(KernelBundleImpl.shared_from_this()),
5052
MIsInterop(MDeviceImageImpl->getOriginMask() & ImageOriginInterop),
5153
MKernelArgMaskPtr{ArgMask}, MCacheMutex{CacheMutex} {
5254
// Enable USM indirect access for interop and non-sycl-jit source kernels.

‎sycl/source/detail/kernel_impl.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class kernel_impl {
4040
/// \param Context is a valid SYCL context
4141
/// \param KernelBundleImpl is a valid instance of kernel_bundle_impl
4242
kernel_impl(ur_kernel_handle_t Kernel, context_impl &Context,
43-
KernelBundleImplPtr KernelBundleImpl,
43+
kernel_bundle_impl *KernelBundleImpl,
4444
const KernelArgMask *ArgMask = nullptr);
4545

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

‎sycl/source/detail/scheduler/commands.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2540,7 +2540,7 @@ getCGKernelInfo(const CGExecKernel &CommandGroup, context_impl &ContextImpl,
25402540
ur_kernel_handle_t UrKernel = nullptr;
25412541
std::shared_ptr<device_image_impl> DeviceImageImpl = nullptr;
25422542
const KernelArgMask *EliminatedArgMask = nullptr;
2543-
auto &KernelBundleImplPtr = CommandGroup.MKernelBundle;
2543+
kernel_bundle_impl *KernelBundleImplPtr = CommandGroup.MKernelBundle.get();
25442544

25452545
if (auto Kernel = CommandGroup.MSyclKernel; Kernel != nullptr) {
25462546
UrKernel = Kernel->getHandleRef();

‎sycl/source/kernel_bundle.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ get_kernel_bundle_impl(const context &Ctx, const std::vector<device> &Devs,
213213
detail::KernelBundleImplPtr
214214
get_kernel_bundle_impl(const context &Ctx, const std::vector<device> &Devs,
215215
const sycl::span<char> &Bytes, bundle_state State) {
216-
return std::make_shared<detail::kernel_bundle_impl>(Ctx, Devs, Bytes, State);
216+
return detail::kernel_bundle_impl::create(Ctx, Devs, Bytes, State);
217217
}
218218

219219
detail::KernelBundleImplPtr
@@ -524,8 +524,8 @@ obj_kb compile_from_source(
524524
LogPtr = &Log;
525525
std::vector<device> UniqueDevices =
526526
sycl::detail::removeDuplicateDevices(Devices);
527-
std::shared_ptr<kernel_bundle_impl> sourceImpl = getSyclObjImpl(SourceKB);
528-
std::shared_ptr<kernel_bundle_impl> KBImpl = sourceImpl->compile_from_source(
527+
kernel_bundle_impl &sourceImpl = *getSyclObjImpl(SourceKB);
528+
std::shared_ptr<kernel_bundle_impl> KBImpl = sourceImpl.compile_from_source(
529529
UniqueDevices, BuildOptions, LogPtr, RegisteredKernelNames);
530530
auto result = sycl::detail::createSyclObjFromImpl<obj_kb>(KBImpl);
531531
if (LogView)
@@ -548,9 +548,8 @@ exe_kb build_from_source(
548548
LogPtr = &Log;
549549
std::vector<device> UniqueDevices =
550550
sycl::detail::removeDuplicateDevices(Devices);
551-
const std::shared_ptr<kernel_bundle_impl> &sourceImpl =
552-
getSyclObjImpl(SourceKB);
553-
std::shared_ptr<kernel_bundle_impl> KBImpl = sourceImpl->build_from_source(
551+
kernel_bundle_impl &sourceImpl = *getSyclObjImpl(SourceKB);
552+
std::shared_ptr<kernel_bundle_impl> KBImpl = sourceImpl.build_from_source(
554553
UniqueDevices, BuildOptions, LogPtr, RegisteredKernelNames);
555554
auto result = sycl::detail::createSyclObjFromImpl<exe_kb>(std::move(KBImpl));
556555
if (LogView)

0 commit comments

Comments
 (0)