From a88c64f86d90a96f575e7c9a5dddc84209e45e0e Mon Sep 17 00:00:00 2001 From: sdashmiz Date: Fri, 5 May 2023 16:30:32 -0400 Subject: [PATCH] SWDEV-367877 - Detect cycle in graph - detect cycle when graph is instantiated - remove level calculation from add/remove node Signed-off-by: sdashmiz Change-Id: I3f7432f91f70aec8e4fd866b2766256f8a9a0cfe graph-cycle-corrections Signed-off-by: sdashmiz Change-Id: I8a3cec9a5a503aac6ea1e85ff3dd2b972790fb1d [ROCm/clr commit: 8578da8a3d0d4c78159239dadfecbd98b1ef4a7e] --- projects/clr/hipamd/src/hip_graph.cpp | 18 ++-- .../clr/hipamd/src/hip_graph_internal.cpp | 56 ++++++------ .../clr/hipamd/src/hip_graph_internal.hpp | 85 ++++++------------- 3 files changed, 67 insertions(+), 92 deletions(-) diff --git a/projects/clr/hipamd/src/hip_graph.cpp b/projects/clr/hipamd/src/hip_graph.cpp index 42f9039a50..9e695287a9 100644 --- a/projects/clr/hipamd/src/hip_graph.cpp +++ b/projects/clr/hipamd/src/hip_graph.cpp @@ -1242,11 +1242,13 @@ hipError_t ihipGraphInstantiate(hipGraphExec_t* pGraphExec, hipGraph_t graph, std::unordered_map> nodeWaitLists; std::unordered_set graphExeUserObj; clonedGraph->GetRunList(parallelLists, nodeWaitLists); - std::vector levelOrder; - clonedGraph->LevelOrder(levelOrder); + std::vector graphNodes; + if (false == clonedGraph->TopologicalOrder(graphNodes)) { + return hipErrorInvalidValue; + } clonedGraph->GetUserObjs(graphExeUserObj); *pGraphExec = - new hipGraphExec(levelOrder, parallelLists, nodeWaitLists, clonedNodes, + new hipGraphExec(graphNodes, parallelLists, nodeWaitLists, clonedNodes, graphExeUserObj, flags); if (*pGraphExec != nullptr) { graph->SetGraphInstantiated(true); @@ -1320,7 +1322,9 @@ hipError_t hipGraphGetNodes(hipGraph_t graph, hipGraphNode_t* nodes, size_t* num HIP_RETURN(hipErrorInvalidValue); } std::vector graphNodes; - graph->LevelOrder(graphNodes); + if (false == graph->TopologicalOrder(graphNodes)) { + HIP_RETURN(hipErrorInvalidValue); + } if (nodes == nullptr) { *numNodes = graphNodes.size(); HIP_RETURN(hipSuccess); @@ -1561,10 +1565,10 @@ hipError_t hipGraphExecChildGraphNodeSetParams(hipGraphExec_t hGraphExec, hipGra // Validate whether the topology of node and childGraph matches std::vector childGraphNodes1; - node->LevelOrder(childGraphNodes1); + node->TopologicalOrder(childGraphNodes1); std::vector childGraphNodes2; - childGraph->LevelOrder(childGraphNodes2); + childGraph->TopologicalOrder(childGraphNodes2); if (childGraphNodes1.size() != childGraphNodes2.size()) { HIP_RETURN(hipErrorUnknown); @@ -2148,7 +2152,7 @@ hipError_t hipGraphExecUpdate(hipGraphExec_t hGraphExec, hipGraph_t hGraph, } std::vector newGraphNodes; - hGraph->LevelOrder(newGraphNodes); + hGraph->TopologicalOrder(newGraphNodes); std::vector& oldGraphExecNodes = hGraphExec->GetNodes(); if (newGraphNodes.size() != oldGraphExecNodes.size()) { *updateResult_out = hipGraphExecUpdateErrorTopologyChanged; diff --git a/projects/clr/hipamd/src/hip_graph_internal.cpp b/projects/clr/hipamd/src/hip_graph_internal.cpp index c4c99f4936..a985218a2c 100644 --- a/projects/clr/hipamd/src/hip_graph_internal.cpp +++ b/projects/clr/hipamd/src/hip_graph_internal.cpp @@ -392,31 +392,32 @@ void ihipGraph::GetRunList(std::vector>& parallelLists, } } } - -void ihipGraph::LevelOrder(std::vector& levelOrder) { - std::vector roots = GetRootNodes(); - std::unordered_map visited; +bool ihipGraph::TopologicalOrder(std::vector& TopoOrder) { std::queue q; - for (auto it = roots.begin(); it != roots.end(); it++) { - q.push(*it); - ClPrint(amd::LOG_INFO, amd::LOG_CODE, "[hipGraph] %s(%p) level:%d \n", - GetGraphNodeTypeString((*it)->GetType()), *it, (*it)->GetLevel()); + std::unordered_map inDegree; + for (auto entry : vertices_) { + if (entry->GetInDegree() == 0) { + q.push(entry); + } + inDegree[entry] = entry->GetInDegree(); } - while (!q.empty()) { + while (!q.empty()) + { Node node = q.front(); + TopoOrder.push_back(node); q.pop(); - levelOrder.push_back(node); - for (const auto& i : node->GetEdges()) { - if (visited.find(i) == visited.end() && i->GetLevel() == (node->GetLevel() + 1)) { - q.push(i); - ClPrint(amd::LOG_INFO, amd::LOG_CODE, "[hipGraph] %s(%p) level:%d \n", - GetGraphNodeTypeString(i->GetType()), i, i->GetLevel()); - visited[i] = true; + for (auto edge : node->GetEdges()) { + inDegree[edge]--; + if (inDegree[edge] == 0) { + q.push(edge); } } } + if (GetNodeCount() == TopoOrder.size()) { + return true; + } + return false; } - ihipGraph* ihipGraph::clone(std::unordered_map& clonedNodes) const { ihipGraph* newGraph = new ihipGraph(device_, this); for (auto entry : vertices_) { @@ -481,8 +482,11 @@ hipError_t hipGraphExec::Init() { hipError_t status; size_t min_num_streams = 1; - for (auto& node : levelOrder_) { - min_num_streams += node->GetNumParallelStreams(); + for (auto& node : topoOrder_) { + status = node->GetNumParallelStreams(min_num_streams); + if(status != hipSuccess) { + return status; + } } status = CreateStreams(parallelLists_.size() - 1 + min_num_streams); return status; @@ -490,10 +494,10 @@ hipError_t hipGraphExec::Init() { hipError_t FillCommands(std::vector>& parallelLists, std::unordered_map>& nodeWaitLists, - std::vector& levelOrder, std::vector& rootCommands, + std::vector& topoOrder, std::vector& rootCommands, amd::Command*& endCommand, hip::Stream* stream) { hipError_t status; - for (auto& node : levelOrder) { + for (auto& node : topoOrder) { // TODO: clone commands from next launch status = node->CreateCommand(node->GetQueue()); if (status != hipSuccess) return status; @@ -575,14 +579,14 @@ hipError_t hipGraphExec::Run(hipStream_t stream) { return hipErrorInvalidResourceHandle; } if (flags_ & hipGraphInstantiateFlagAutoFreeOnLaunch) { - if (!levelOrder_.empty()) { - levelOrder_[0]->GetParentGraph()->FreeAllMemory(); + if (!topoOrder_.empty()) { + topoOrder_[0]->GetParentGraph()->FreeAllMemory(); } } // If this is a repeat launch, make sure corresponding MemFreeNode exists for a MemAlloc node if (repeatLaunch_ == true) { - for (auto& node : levelOrder_) { + for (auto& node : topoOrder_) { if (node->GetType() == hipGraphNodeTypeMemAlloc && static_cast(node)->IsActiveMem() == true) { return hipErrorInvalidValue; @@ -599,7 +603,7 @@ hipError_t hipGraphExec::Run(hipStream_t stream) { std::vector rootCommands; amd::Command* endCommand = nullptr; status = - FillCommands(parallelLists_, nodeWaitLists_, levelOrder_, rootCommands, endCommand, hip_stream); + FillCommands(parallelLists_, nodeWaitLists_, topoOrder_, rootCommands, endCommand, hip_stream); if (status != hipSuccess) { return status; } @@ -607,7 +611,7 @@ hipError_t hipGraphExec::Run(hipStream_t stream) { cmd->enqueue(); cmd->release(); } - for (auto& node : levelOrder_) { + for (auto& node : topoOrder_) { node->EnqueueCommands(stream); } if (endCommand != nullptr) { diff --git a/projects/clr/hipamd/src/hip_graph_internal.hpp b/projects/clr/hipamd/src/hip_graph_internal.hpp index 04085249cd..43dd4e4202 100644 --- a/projects/clr/hipamd/src/hip_graph_internal.hpp +++ b/projects/clr/hipamd/src/hip_graph_internal.hpp @@ -38,7 +38,7 @@ typedef hipGraphNode* Node; hipError_t FillCommands(std::vector>& parallelLists, std::unordered_map>& nodeWaitLists, - std::vector& levelOrder, std::vector& rootCommands, + std::vector& topoOrder, std::vector& rootCommands, amd::Command*& endCommand, hip::Stream* stream); void UpdateStream(std::vector>& parallelLists, hip::Stream* stream, hipGraphExec* ptr); @@ -157,7 +157,6 @@ struct hipGraphNodeDOTAttribute { struct hipGraphNode : public hipGraphNodeDOTAttribute { protected: hip::Stream* stream_ = nullptr; - uint32_t level_; unsigned int id_; hipGraphNodeType type_; std::vector commands_; @@ -178,7 +177,6 @@ struct hipGraphNode : public hipGraphNodeDOTAttribute { hipGraphNode(hipGraphNodeType type, std::string style = "", std::string shape = "", std::string label = "") : type_(type), - level_(0), visited_(false), inDegree_(0), outDegree_(0), @@ -191,7 +189,6 @@ struct hipGraphNode : public hipGraphNodeDOTAttribute { } /// Copy Constructor hipGraphNode(const hipGraphNode& node) : hipGraphNodeDOTAttribute(node) { - level_ = node.level_; type_ = node.type_; inDegree_ = node.inDegree_; outDegree_ = node.outDegree_; @@ -240,10 +237,6 @@ struct hipGraphNode : public hipGraphNodeDOTAttribute { virtual std::vector& GetCommands() { return commands_; } /// Returns graph node type hipGraphNodeType GetType() const { return type_; } - /// Returns graph node in coming edges - uint32_t GetLevel() const { return level_; } - /// Set graph node level - void SetLevel(uint32_t level) { level_ = level; } /// Clone graph node virtual hipGraphNode* clone() const = 0; /// Returns graph node indegree @@ -280,29 +273,14 @@ struct hipGraphNode : public hipGraphNodeDOTAttribute { edges_.push_back(entry); } } - /// Update level, for existing edges - void UpdateEdgeLevel() { - for (auto edge : edges_) { - edge->SetLevel(std::max(edge->GetLevel(), GetLevel() + 1)); - edge->UpdateEdgeLevel(); - } - } - void ReduceEdgeLevel() { - for (auto edge: edges_) { - edge->SetLevel(std::min(edge->GetLevel(),GetLevel() + 1)); - edge->ReduceEdgeLevel(); - } - } - /// Add edge, update parent node outdegree, child node indegree, level and dependency + /// Add edge, update parent node outdegree, child node indegree and dependency void AddEdge(const Node& childNode) { edges_.push_back(childNode); outDegree_++; childNode->SetInDegree(childNode->GetInDegree() + 1); - childNode->SetLevel(std::max(childNode->GetLevel(), GetLevel() + 1)); - childNode->UpdateEdgeLevel(); childNode->AddDependency(this); } - /// Remove edge, update parent node outdegree, child node indegree, level and dependency + /// Remove edge, update parent node outdegree, child node indegree and dependency bool RemoveUpdateEdge(const Node& childNode) { // std::remove changes the end() hence saving it before hand for validation auto currEdgeEnd = edges_.end(); @@ -315,33 +293,20 @@ struct hipGraphNode : public hipGraphNodeDOTAttribute { outDegree_--; childNode->SetInDegree(childNode->GetInDegree() - 1); childNode->RemoveDependency(this); - const std::vector& dependencies = childNode->GetDependencies(); - int32_t level = 0; - int32_t parentLevel = 0; - uint32_t origLevel = 0; - for (auto parent : dependencies) { - parentLevel = parent->GetLevel(); - level = std::max(level, (parentLevel + 1)); - } - origLevel = childNode->GetLevel(); - childNode->SetLevel(level); - if (level < origLevel) { - childNode->ReduceEdgeLevel(); - } return true; } /// Get Runlist of the nodes embedded as part of the graphnode(e.g. ChildGraph) virtual void GetRunList(std::vector>& parallelList, std::unordered_map>& dependencies) {} - /// Get levelorder of the nodes embedded as part of the graphnode(e.g. ChildGraph) - virtual void LevelOrder(std::vector& levelOrder) {} + /// Get topological sort of the nodes embedded as part of the graphnode(e.g. ChildGraph) + virtual bool TopologicalOrder(std::vector& TopoOrder) { return true; } /// Update waitlist of the nodes embedded as part of the graphnode(e.g. ChildGraph) virtual void UpdateEventWaitLists(amd::Command::EventWaitList waitList) { for (auto command : commands_) { command->updateEventWaitList(waitList); } } - virtual size_t GetNumParallelStreams() { return 0; } + virtual hipError_t GetNumParallelStreams(size_t &num) { return hipSuccess; } /// Enqueue commands part of the node virtual void EnqueueCommands(hipStream_t stream) { // If the node is disabled it becomes empty node. To maintain ordering just enqueue marker. @@ -479,7 +444,7 @@ struct ihipGraph { std::unordered_map>& dependencies); void GetRunList(std::vector>& parallelLists, std::unordered_map>& dependencies); - void LevelOrder(std::vector& levelOrder); + bool TopologicalOrder(std::vector& TopoOrder); void GetUserObjs(std::unordered_set& graphExeUserObjs) { for (auto userObj : graphUserObj_) { userObj->retain(); @@ -575,8 +540,8 @@ struct ihipGraph { struct hipGraphExec { std::vector> parallelLists_; - // level order of the graph doesn't include nodes embedded as part of the child graph - std::vector levelOrder_; + // Topological order of the graph doesn't include nodes embedded as part of the child graph + std::vector topoOrder_; std::unordered_map> nodeWaitLists_; std::vector parallel_streams_; uint currentQueueIndex_; @@ -588,13 +553,13 @@ struct hipGraphExec { uint64_t flags_ = 0; bool repeatLaunch_ = false; public: - hipGraphExec(std::vector& levelOrder, std::vector>& lists, + hipGraphExec(std::vector& topoOrder, std::vector>& lists, std::unordered_map>& nodeWaitLists, std::unordered_map& clonedNodes, std::unordered_set& userObjs, uint64_t flags = 0) : parallelLists_(lists), - levelOrder_(levelOrder), + topoOrder_(topoOrder), nodeWaitLists_(nodeWaitLists), clonedNodes_(clonedNodes), lastEnqueuedCommand_(nullptr), @@ -634,7 +599,7 @@ struct hipGraphExec { // check executable graphs validity static bool isGraphExecValid(hipGraphExec* pGraphExec); - std::vector& GetNodes() { return levelOrder_; } + std::vector& GetNodes() { return topoOrder_; } hip::Stream* GetAvailableStreams() { return parallel_streams_[currentQueueIndex_++]; } void ResetQueueIndex() { currentQueueIndex_ = 0; } @@ -645,7 +610,7 @@ struct hipGraphExec { struct hipChildGraphNode : public hipGraphNode { struct ihipGraph* childGraph_; - std::vector childGraphlevelOrder_; + std::vector childGraphNodeOrder_; std::vector> parallelLists_; std::unordered_map> nodeWaitLists_; amd::Command* lastEnqueuedCommand_; @@ -668,15 +633,19 @@ struct hipChildGraphNode : public hipGraphNode { ihipGraph* GetChildGraph() { return childGraph_; } - size_t GetNumParallelStreams() { - LevelOrder(childGraphlevelOrder_); - size_t num = 0; - for (auto& node : childGraphlevelOrder_) { - num += node->GetNumParallelStreams(); + hipError_t GetNumParallelStreams(size_t &num) { + if (false == TopologicalOrder(childGraphNodeOrder_)) { + return hipErrorInvalidValue; + } + for (auto& node : childGraphNodeOrder_) { + if (hipSuccess != node->GetNumParallelStreams(num)) { + return hipErrorInvalidValue; + } } // returns total number of parallel queues required for child graph nodes to be launched // first parallel list will be launched on the same queue as parent - return num + (parallelLists_.size() - 1); + num += (parallelLists_.size() - 1); + return hipSuccess; } void SetStream(hip::Stream* stream, hipGraphExec* ptr = nullptr) { @@ -697,7 +666,7 @@ struct hipChildGraphNode : public hipGraphNode { commands_.reserve(2); std::vector rootCommands; amd::Command* endCommand = nullptr; - status = FillCommands(parallelLists_, nodeWaitLists_, childGraphlevelOrder_, rootCommands, + status = FillCommands(parallelLists_, nodeWaitLists_, childGraphNodeOrder_, rootCommands, endCommand, stream); for (auto& cmd : rootCommands) { commands_.push_back(cmd); @@ -717,9 +686,7 @@ struct hipChildGraphNode : public hipGraphNode { std::unordered_map>& dependencies) { childGraph_->GetRunList(parallelLists_, nodeWaitLists_); } - - void LevelOrder(std::vector& levelOrder) { childGraph_->LevelOrder(levelOrder); } - + bool TopologicalOrder(std::vector& TopoOrder) { return childGraph_->TopologicalOrder(TopoOrder); } void EnqueueCommands(hipStream_t stream) { // enqueue child graph start command if (commands_.size() == 1) { @@ -727,7 +694,7 @@ struct hipChildGraphNode : public hipGraphNode { commands_[0]->release(); } // enqueue nodes in child graph in level order - for (auto& node : childGraphlevelOrder_) { + for (auto& node : childGraphNodeOrder_) { node->EnqueueCommands(stream); } // enqueue child graph end command