Skip to content

Commit b5d5aad

Browse files
authored
[SYCL][Fusion] Improve circular dependency detection (#8148)
Kernel fusion can potentially create cycles in the SYCL dependency graph. These potential cycles must be detected and fusion must be cancelled, if it would give rise to such a cycle. This PR improves the detection process for cycles to traverse the dependency graph. As this is more effort than the old solution, cycle detection has been moved from `queue::submit`, where it would be happening for each kernel submission to a queue in fusion mode, to `fusion_wrapper::complete_fusion`, where it happens at most once per fusion. Signed-off-by: Lukas Sommer <[email protected]>
1 parent 33cd6b1 commit b5d5aad

File tree

1 file changed

+78
-18
lines changed

1 file changed

+78
-18
lines changed

sycl/source/detail/scheduler/graph_builder.cpp

Lines changed: 78 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <sycl/access/access.hpp>
2121
#include <sycl/exception.hpp>
2222

23+
#include <algorithm>
2324
#include <cstdlib>
2425
#include <cstring>
2526
#include <fstream>
@@ -985,22 +986,6 @@ Scheduler::GraphBuilder::addCG(std::unique_ptr<detail::CG> CommandGroup,
985986
createGraphForCommand(NewCmd.get(), NewCmd->getCG(),
986987
isInteropHostTask(NewCmd.get()), Reqs, Events, Queue,
987988
FusionCmd->auxiliaryCommands());
988-
// We need to check the commands that this kernel depends on for any other
989-
// commands that have been submitted to another queue which is also in
990-
// fusion mode. If we detect such another command, we cancel fusion for that
991-
// other queue to avoid circular dependencies.
992-
// Handle requirements on any commands part of another active fusion.
993-
for (auto &Dep : NewCmd->MDeps) {
994-
auto *DepCmd = Dep.MDepCommand;
995-
if (!DepCmd) {
996-
continue;
997-
}
998-
if (DepCmd->getQueue() != Queue && isPartOfActiveFusion(DepCmd)) {
999-
printFusionWarning("Aborting fusion because of requirement from a "
1000-
"different fusion process");
1001-
cancelFusion(DepCmd->getQueue(), ToEnqueue);
1002-
}
1003-
}
1004989

1005990
// Set the fusion command, so we recognize when another command depends on a
1006991
// kernel in the fusion list.
@@ -1431,6 +1416,63 @@ void Scheduler::GraphBuilder::cancelFusion(QueueImplPtr Queue,
14311416
PlaceholderCmd->setFusionStatus(KernelFusionCommand::FusionStatus::CANCELLED);
14321417
}
14331418

1419+
static bool isPartOfFusion(Command *Cmd, KernelFusionCommand *Fusion) {
1420+
if (Cmd->getType() == Command::RUN_CG) {
1421+
return static_cast<ExecCGCommand *>(Cmd)->MFusionCmd == Fusion;
1422+
}
1423+
return false;
1424+
}
1425+
1426+
static bool checkForCircularDependency(Command *, bool, KernelFusionCommand *);
1427+
1428+
static bool createsCircularDependency(Command *Cmd, bool PredPartOfFusion,
1429+
KernelFusionCommand *Fusion) {
1430+
if (isPartOfFusion(Cmd, Fusion)) {
1431+
// If this is part of the fusion and the predecessor also was, we can stop
1432+
// the traversal here. A direct dependency between two kernels in the same
1433+
// fusion will never form a cyclic dependency and by iterating over all
1434+
// commands in a fusion, we will detect any cycles originating from the
1435+
// current command.
1436+
// If the predecessor was not part of the fusion, but the current command
1437+
// is, we have found a potential cycle in the dependency graph.
1438+
return !PredPartOfFusion;
1439+
}
1440+
return checkForCircularDependency(Cmd, false, Fusion);
1441+
}
1442+
1443+
static bool checkForCircularDependency(Command *Cmd, bool IsPartOfFusion,
1444+
KernelFusionCommand *Fusion) {
1445+
// Check the requirement dependencies.
1446+
for (auto &Dep : Cmd->MDeps) {
1447+
auto *DepCmd = Dep.MDepCommand;
1448+
if (!DepCmd) {
1449+
continue;
1450+
}
1451+
if (createsCircularDependency(DepCmd, IsPartOfFusion, Fusion)) {
1452+
return true;
1453+
}
1454+
}
1455+
for (auto &Ev : Cmd->getPreparedDepsEvents()) {
1456+
auto *EvDepCmd = static_cast<Command *>(Ev->getCommand());
1457+
if (!EvDepCmd) {
1458+
continue;
1459+
}
1460+
if (createsCircularDependency(EvDepCmd, IsPartOfFusion, Fusion)) {
1461+
return true;
1462+
}
1463+
}
1464+
for (auto &Ev : Cmd->getPreparedHostDepsEvents()) {
1465+
auto *EvDepCmd = static_cast<Command *>(Ev->getCommand());
1466+
if (!EvDepCmd) {
1467+
continue;
1468+
}
1469+
if (createsCircularDependency(EvDepCmd, IsPartOfFusion, Fusion)) {
1470+
return true;
1471+
}
1472+
}
1473+
return false;
1474+
}
1475+
14341476
EventImplPtr
14351477
Scheduler::GraphBuilder::completeFusion(QueueImplPtr Queue,
14361478
std::vector<Command *> &ToEnqueue,
@@ -1451,8 +1493,26 @@ Scheduler::GraphBuilder::completeFusion(QueueImplPtr Queue,
14511493
auto *PlaceholderCmd = FusionList->second.get();
14521494
auto &CmdList = PlaceholderCmd->getFusionList();
14531495

1454-
// TODO: The logic to invoke the JIT compiler to create a fused kernel from
1455-
// the list will be added in a later PR.
1496+
// We need to check if fusing the kernel would create a circular dependency. A
1497+
// circular dependency would arise, if a kernel in the fusion list
1498+
// *indirectly* depends on another kernel in the fusion list. Here, indirectly
1499+
// means, that the dependency is created through a third command not part of
1500+
// the fusion, on which this kernel depends and which in turn depends on
1501+
// another kernel in fusion list.
1502+
bool CreatesCircularDep =
1503+
std::any_of(CmdList.begin(), CmdList.end(), [&](ExecCGCommand *Cmd) {
1504+
return checkForCircularDependency(Cmd, true, PlaceholderCmd);
1505+
});
1506+
if (CreatesCircularDep) {
1507+
// If fusing would create a fused kernel, cancel the fusion.
1508+
printFusionWarning(
1509+
"Aborting fusion because it would create a circular dependency");
1510+
auto LastEvent = PlaceholderCmd->getEvent();
1511+
this->cancelFusion(Queue, ToEnqueue);
1512+
return LastEvent;
1513+
}
1514+
1515+
// Call the JIT compiler to generate a new fused kernel.
14561516
auto FusedCG = detail::jit_compiler::get_instance().fuseKernels(
14571517
Queue, CmdList, PropList);
14581518

0 commit comments

Comments
 (0)