Skip to content

Commit ded227d

Browse files
authored
[SYCL] Don't perform early free of native kernel command group (#2674)
Signed-off-by: Sergey Kanaev <[email protected]>
1 parent ffdadc2 commit ded227d

File tree

4 files changed

+55
-5
lines changed

4 files changed

+55
-5
lines changed

sycl/source/detail/scheduler/commands.cpp

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1724,12 +1724,18 @@ pi_result ExecCGCommand::SetKernelParamsAndLaunch(
17241724
void DispatchNativeKernel(void *Blob) {
17251725
// First value is a pointer to Corresponding CGExecKernel object.
17261726
CGExecKernel *HostTask = *(CGExecKernel **)Blob;
1727+
bool ShouldDeleteCG = static_cast<void **>(Blob)[1] != nullptr;
17271728

17281729
// Other value are pointer to the buffers.
1729-
void **NextArg = (void **)Blob + 1;
1730+
void **NextArg = static_cast<void **>(Blob) + 2;
17301731
for (detail::Requirement *Req : HostTask->MRequirements)
17311732
Req->MData = *(NextArg++);
17321733
HostTask->MHostKernel->call(HostTask->MNDRDesc, nullptr);
1734+
1735+
// The command group will (if not already was) be released in scheduler.
1736+
// Hence we're free to deallocate it here.
1737+
if (ShouldDeleteCG)
1738+
delete HostTask;
17331739
}
17341740

17351741
cl_int ExecCGCommand::enqueueImp() {
@@ -1814,9 +1820,14 @@ cl_int ExecCGCommand::enqueueImp() {
18141820
// piEnqueueNativeKernel takes arguments blob which is passes to user
18151821
// function.
18161822
// Reserve extra space for the pointer to CGExecKernel to restore context.
1817-
std::vector<void *> ArgsBlob(HostTask->MArgs.size() + 1);
1823+
std::vector<void *> ArgsBlob(HostTask->MArgs.size() + 2);
18181824
ArgsBlob[0] = (void *)HostTask;
1819-
void **NextArg = ArgsBlob.data() + 1;
1825+
{
1826+
std::intptr_t ShouldDeleteCG =
1827+
static_cast<std::intptr_t>(MDeps.size() == 0 && MUsers.size() == 0);
1828+
ArgsBlob[1] = reinterpret_cast<void *>(ShouldDeleteCG);
1829+
}
1830+
void **NextArg = ArgsBlob.data() + 2;
18201831

18211832
if (MQueue->is_host()) {
18221833
for (ArgDesc &Arg : HostTask->MArgs) {

sycl/source/detail/scheduler/commands.hpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -505,6 +505,15 @@ class ExecCGCommand : public Command {
505505
// the cleanup process.
506506
EmptyCommand *MEmptyCmd = nullptr;
507507

508+
// This function is only usable for native kernel to prevent access to free'd
509+
// memory in DispatchNativeKernel.
510+
// TODO remove when native kernel support is terminated.
511+
void releaseCG() {
512+
assert(MCommandGroup->getType() == CG::RUN_ON_HOST_INTEL &&
513+
"Only 'native kernel' is allowed to release command group");
514+
MCommandGroup.release();
515+
}
516+
508517
private:
509518
cl_int enqueueImp() final override;
510519

sycl/source/detail/scheduler/scheduler.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ EventImplPtr Scheduler::addCG(std::unique_ptr<detail::CG> CommandGroup,
7070
QueueImplPtr Queue) {
7171
EventImplPtr NewEvent = nullptr;
7272
const bool IsKernel = CommandGroup->getType() == CG::KERNEL;
73+
const bool IsHostKernel = CommandGroup->getType() == CG::RUN_ON_HOST_INTEL;
7374
vector_class<StreamImplPtr> Streams;
7475
{
7576
std::unique_lock<std::shared_timed_mutex> Lock(MGraphLock, std::defer_lock);
@@ -104,9 +105,15 @@ EventImplPtr Scheduler::addCG(std::unique_ptr<detail::CG> CommandGroup,
104105
if (IsKernel)
105106
Streams = ((ExecCGCommand *)NewCmd)->getStreams();
106107

108+
// If there are no memory dependencies decouple and free the command.
109+
// Though, dismiss ownership of native kernel command group as it's
110+
// resources may be in use by backend and synchronization point here is
111+
// at native kernel execution finish.
107112
if (NewCmd->MDeps.size() == 0 && NewCmd->MUsers.size() == 0) {
108-
NewEvent->setCommand(nullptr); // if there are no memory dependencies,
109-
// decouple and free the command
113+
if (IsHostKernel)
114+
static_cast<ExecCGCommand *>(NewCmd)->releaseCG();
115+
116+
NewEvent->setCommand(nullptr);
110117
delete NewCmd;
111118
}
112119
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// RUN: %clangxx -fsycl %s -o %t.out
2+
// RUN: env SYCL_DEVICE_TYPE=HOST %t.out
3+
// RUN: %CPU_RUN_PLACEHOLDER %t.out
4+
5+
#include <CL/sycl.hpp>
6+
7+
// This tests that early free of command (and, hence, the command group) won't
8+
// affect "native kernel" feature support.
9+
int main(void) {
10+
cl::sycl::queue Q;
11+
12+
int *Ptr = new int;
13+
14+
auto E = Q.submit([&](cl::sycl::handler &CGH) {
15+
CGH.run_on_host_intel([=] { *Ptr = 5; });
16+
});
17+
18+
E.wait();
19+
20+
std::cout << "Finished successfully\n";
21+
22+
return 0;
23+
}

0 commit comments

Comments
 (0)