Skip to content

Commit 549ad94

Browse files
aelovikov-intelaelovikov-intel
committed
[NFC][SYCL] More cleanup after enable_shared_from_this for kernel_bundle_impl
Follow-up for intel#18899. Also adds proper `private_tag` argument for the ctor added in intel#18949 that missed that. Also pass `span` by value while on it.
1 parent aaab7e9 commit 549ad94

File tree

8 files changed

+27
-30
lines changed

8 files changed

+27
-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: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ std::shared_ptr<kernel_impl> device_image_impl::tryGetExtensionKernel(
3535
PM.getOrCreateKernel(Context, AdjustedName,
3636
/*PropList=*/{}, UrProgram);
3737
return std::make_shared<kernel_impl>(UrKernel, *getSyclObjImpl(Context),
38-
Self, OwnerBundle.shared_from_this(),
39-
ArgMask, UrProgram, CacheMutex);
38+
Self, OwnerBundle, ArgMask,
39+
UrProgram, CacheMutex);
4040
}
4141
return nullptr;
4242
}
@@ -49,8 +49,7 @@ std::shared_ptr<kernel_impl> device_image_impl::tryGetExtensionKernel(
4949
// Kernel created by urKernelCreate is implicitly retained.
5050

5151
return std::make_shared<kernel_impl>(
52-
UrKernel, *detail::getSyclObjImpl(Context), Self,
53-
OwnerBundle.shared_from_this(),
52+
UrKernel, *detail::getSyclObjImpl(Context), Self, OwnerBundle,
5453
/*ArgMask=*/nullptr, UrProgram, /*CacheMutex=*/nullptr);
5554
}
5655

‎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

@@ -994,9 +994,8 @@ class kernel_bundle_impl
994994
SelectedImage->get_ur_program_ref());
995995

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

10021001
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
@@ -2536,7 +2536,7 @@ getCGKernelInfo(const CGExecKernel &CommandGroup, context_impl &ContextImpl,
25362536
ur_kernel_handle_t UrKernel = nullptr;
25372537
std::shared_ptr<device_image_impl> DeviceImageImpl = nullptr;
25382538
const KernelArgMask *EliminatedArgMask = nullptr;
2539-
auto &KernelBundleImplPtr = CommandGroup.MKernelBundle;
2539+
kernel_bundle_impl *KernelBundleImplPtr = CommandGroup.MKernelBundle.get();
25402540

25412541
if (auto Kernel = CommandGroup.MSyclKernel; Kernel != nullptr) {
25422542
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)