Skip to content

[SYCL] Try to enqueue host command depencies #2561

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.

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

Merged
merged 21 commits into from
Oct 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion sycl/source/detail/scheduler/commands.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1737,7 +1737,8 @@ void DispatchNativeKernel(void *Blob) {
}

cl_int ExecCGCommand::enqueueImp() {
waitForPreparedHostEvents();
if (getCG().getType() != CG::CGTYPE::CODEPLAY_HOST_TASK)
waitForPreparedHostEvents();
std::vector<EventImplPtr> EventImpls = MPreparedDepsEvents;
auto RawEvents = getPiEvents(EventImpls);

Expand Down
4 changes: 4 additions & 0 deletions sycl/source/detail/scheduler/commands.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,10 @@ class Command {
friend class DispatchHostTask;

public:
const std::vector<EventImplPtr> getPreparedHostDepsEvents() const {
return MPreparedHostDepsEvents;
}

/// Contains list of dependencies(edges)
std::vector<DepDesc> MDeps;
/// Contains list of commands that depend on the command.
Expand Down
16 changes: 16 additions & 0 deletions sycl/source/detail/scheduler/graph_processor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,22 @@ bool Scheduler::GraphProcessor::enqueueCommand(Command *Cmd,
return false;
}

// Asynchronous host operations (amongst dependencies of an arbitrary command)
// are not supported (see Command::processDepEvent method). This impacts
// operation of host-task feature a lot with hangs and long-runs. Hence we
// have this workaround here.
// This workaround is safe as long as the only asynchronous host operation we
// have is a host task.
// This may iterate over some of dependencies in Cmd->MDeps. Though, the
// enqueue operation is idempotent and the second call will result in no-op.
// TODO remove the workaround when proper fix for host-task dispatching is
// implemented.
for (const EventImplPtr &Event : Cmd->getPreparedHostDepsEvents()) {
if (Command *DepCmd = static_cast<Command *>(Event->getCommand()))
if (!enqueueCommand(DepCmd, EnqueueResult, Blocking))
return false;
}

return Cmd->enqueue(EnqueueResult, Blocking);
}

Expand Down
12 changes: 6 additions & 6 deletions sycl/test/host-interop-task/host-task-dependency2.cpp
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t.out

// RUNx: %CPU_RUN_PLACEHOLDER %t.out
// RUNx: %GPU_RUN_PLACEHOLDER %t.out
// RUNx: %ACC_RUN_PLACEHOLDER %t.out
// RUN: %CPU_RUN_PLACEHOLDER %t.out
// RUN: %GPU_RUN_PLACEHOLDER %t.out
// RUN: %ACC_RUN_PLACEHOLDER %t.out

// RUNx: %CPU_RUN_PLACEHOLDER %t.out 10
// RUNx: %GPU_RUN_PLACEHOLDER %t.out 10
// RUNx: %ACC_RUN_PLACEHOLDER %t.out 10
// RUN: %CPU_RUN_PLACEHOLDER %t.out 10
// RUN: %GPU_RUN_PLACEHOLDER %t.out 10
// RUN: %ACC_RUN_PLACEHOLDER %t.out 10

#include <CL/sycl.hpp>
#include <iostream>
Expand Down
12 changes: 6 additions & 6 deletions sycl/test/host-interop-task/host-task-dependency3.cpp
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t.out

// RUNx: %CPU_RUN_PLACEHOLDER %t.out
// RUNx: %GPU_RUN_PLACEHOLDER %t.out
// RUNx: %ACC_RUN_PLACEHOLDER %t.out
// RUN: %CPU_RUN_PLACEHOLDER %t.out
// RUN: %GPU_RUN_PLACEHOLDER %t.out
// RUN: %ACC_RUN_PLACEHOLDER %t.out

// RUNx: %CPU_RUN_PLACEHOLDER %t.out 10
// RUNx: %GPU_RUN_PLACEHOLDER %t.out 10
// RUNx: %ACC_RUN_PLACEHOLDER %t.out 10
// RUN: %CPU_RUN_PLACEHOLDER %t.out 10
// RUN: %GPU_RUN_PLACEHOLDER %t.out 10
// RUN: %ACC_RUN_PLACEHOLDER %t.out 10

#include <CL/sycl.hpp>
#include <chrono>
Expand Down
6 changes: 3 additions & 3 deletions sycl/test/host-interop-task/host-task-dependency4.cpp
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t.out

// RUNx: %CPU_RUN_PLACEHOLDER %t.out
// RUNx: %GPU_RUN_PLACEHOLDER %t.out
// RUNx: %ACC_RUN_PLACEHOLDER %t.out
// RUN: %CPU_RUN_PLACEHOLDER %t.out
// RUN: %GPU_RUN_PLACEHOLDER %t.out
// RUN: %ACC_RUN_PLACEHOLDER %t.out

#include <CL/sycl.hpp>

Expand Down
39 changes: 39 additions & 0 deletions sycl/unittests/scheduler/BlockedCommands.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,3 +130,42 @@ TEST_F(SchedulerTest, EnqueueBlockedCommandEarlyExit) {
<< "Result of enqueueing blocked command should be BLOCKED.\n";
ASSERT_EQ(&B, Res.MCmd) << "Expected different failed command.\n";
}

// This unit test is for workaround described in GraphProcessor::enqueueCommand
// method.
TEST_F(SchedulerTest, EnqueueHostDependency) {
MockCommand A(detail::getSyclObjImpl(MQueue));
A.MEnqueueStatus = detail::EnqueueResultT::SyclEnqueueReady;
A.MIsBlockable = true;
A.MRetVal = CL_SUCCESS;

MockCommand B(detail::getSyclObjImpl(MQueue));
B.MEnqueueStatus = detail::EnqueueResultT::SyclEnqueueReady;
B.MIsBlockable = true;
B.MRetVal = CL_SUCCESS;

cl::sycl::detail::EventImplPtr DepEvent{
new cl::sycl::detail::event_impl(detail::getSyclObjImpl(MQueue))};
DepEvent->setCommand(&B);

A.addDep(DepEvent);

// We have such a "graph":
//
// A
// |
// B
//
// A depends on B. B is host command.
// "Graph" is quoted as we don't have this dependency in MDeps. Instead, we
// have this dependecy as result of handler::depends_on() call.

EXPECT_CALL(A, enqueue(_, _)).Times(1);
EXPECT_CALL(B, enqueue(_, _)).Times(1);

detail::EnqueueResultT Res;
bool Enqueued = MockScheduler::enqueueCommand(&A, Res, detail::NON_BLOCKING);
ASSERT_TRUE(Enqueued) << "The command should be enqueued\n";
ASSERT_EQ(detail::EnqueueResultT::SyclEnqueueSuccess, Res.MResult)
<< "Enqueue operation should return successfully.\n";
}