From bcb59eeb3885ccafe5b6b062448a1458547c0840 Mon Sep 17 00:00:00 2001 From: Maxime France-Pillois Date: Mon, 7 Aug 2023 16:50:12 +0100 Subject: [PATCH 1/6] [SYCL][Graph] Duplicate sub-graph nodes Duplicates the nodes of a sub-graph when added to its parents to enable multiple parents. As a result nodes of the initial sub-graph and the one of the main graph are no longer the exact same (shared_pointer) but two different nodes with similar content. A few unitests have been updated accordingly and an equal operator has been added to node_impl to ease comparison between nodes. --- sycl/source/detail/graph_impl.cpp | 53 ++++++++ sycl/source/detail/graph_impl.hpp | 72 +++++++++- sycl/source/handler.cpp | 5 +- .../sub_graph_multiple_submission.cpp | 3 - .../Explicit/sub_graph_two_parent_graphs.cpp | 3 - .../Inputs/sub_graph_multiple_submission.cpp | 2 +- .../Inputs/sub_graph_two_parent_graphs.cpp | 2 +- .../sub_graph_multiple_submission.cpp | 3 - .../sub_graph_two_parent_graphs.cpp | 3 - .../subgraph_interleaved_submit.cpp | 91 ------------- .../subgraph_two_parent_graphs.cpp | 128 ------------------ sycl/unittests/Extensions/CommandGraph.cpp | 36 +++-- 12 files changed, 155 insertions(+), 246 deletions(-) delete mode 100644 sycl/test-e2e/Graph/RecordReplay/subgraph_interleaved_submit.cpp delete mode 100644 sycl/test-e2e/Graph/RecordReplay/subgraph_two_parent_graphs.cpp diff --git a/sycl/source/detail/graph_impl.cpp b/sycl/source/detail/graph_impl.cpp index 4582ded1ab2ce..d37a1ac933a63 100644 --- a/sycl/source/detail/graph_impl.cpp +++ b/sycl/source/detail/graph_impl.cpp @@ -108,6 +108,21 @@ bool visitNodeDepthFirst( NodeStack.pop_back(); return false; } + +/// Recursively append CurrentNode to Outputs if a given node is an exit node +/// @param[in] CurrentNode Node to check as exit node. +/// @param[inout] Outputs list of exit nodes. +void appendExitNodesFromRoot(std::shared_ptr CurrentNode, + std::vector> &Outputs) { + if (CurrentNode->MSuccessors.size() > 0) { + for (auto Successor : CurrentNode->MSuccessors) { + appendExitNodesFromRoot(Successor, Outputs); + } + } else { + Outputs.push_back(CurrentNode); + } +} + } // anonymous namespace void exec_graph_impl::schedule() { @@ -148,6 +163,27 @@ std::shared_ptr graph_impl::addSubgraphNodes( return this->add(Outputs); } +std::shared_ptr graph_impl::DuplicateAndAddSubgraphNodes( + const std::shared_ptr &SubGraph) { + std::map> NodesMaps; + std::vector> SubGraphRoots; + std::vector> SubGraphOutputs; + + for (auto &Root : SubGraph->MRoots) { + auto NewRoot = Root->duplicateNodeAndSuccessors(NodesMaps); + SubGraphRoots.push_back(NewRoot); + appendExitNodesFromRoot(NewRoot, SubGraphOutputs); + } + + // Recursively walk the graph to find exit nodes and connect up the inputs + // TODO: Consider caching exit nodes so we don't have to do this + for (auto NodeImpl : MRoots) { + connectToExitNodes(NodeImpl, SubGraphRoots); + } + + return this->add(SubGraphOutputs); +} + void graph_impl::addRoot(const std::shared_ptr &Root) { MRoots.insert(Root); } @@ -386,6 +422,23 @@ void graph_impl::makeEdge(std::shared_ptr Src, removeRoot(Dest); // remove receiver from root node list } +void graph_impl::duplicateGraph(const graph_impl &GraphImpl) { + std::map> NodesMaps; + for (auto &Root : GraphImpl.MRoots) { + auto CpyRoot = Root->duplicateNodeAndSuccessors(NodesMaps); + this->addRoot(CpyRoot); + } + + for (auto &Queue : GraphImpl.MRecordingQueues) { + MRecordingQueues.insert(Queue); + } + + for (auto &Element : GraphImpl.MEventsMap) { + if (NodesMaps.find(Element.second.get()) != NodesMaps.end()) + addEventForNode(Element.first, NodesMaps[Element.second.get()]); + } +} + // Check if nodes are empty and if so loop back through predecessors until we // find the real dependency. void exec_graph_impl::findRealDeps( diff --git a/sycl/source/detail/graph_impl.hpp b/sycl/source/detail/graph_impl.hpp index 114022fa6cd9e..cf0054a529f55 100644 --- a/sycl/source/detail/graph_impl.hpp +++ b/sycl/source/detail/graph_impl.hpp @@ -92,6 +92,26 @@ class node_impl { std::unique_ptr &&CommandGroup) : MCGType(CGType), MCommandGroup(std::move(CommandGroup)) {} + /// Tests if two nodes have the same content, + /// i.e. same command group + /// @param Node node to compare with + bool operator==(const node_impl &Node) { + if (MCGType != Node.MCGType) + return false; + + if ((MCGType == sycl::detail::CG::CGTYPE::Kernel) && + (Node.MCGType == sycl::detail::CG::CGTYPE::Kernel)) { + sycl::detail::CGExecKernel *ExecKernelA = + static_cast(MCommandGroup.get()); + sycl::detail::CGExecKernel *ExecKernelB = + static_cast(Node.MCommandGroup.get()); + + if (ExecKernelA->MKernelName.compare(ExecKernelB->MKernelName) != 0) + return false; + } + return true; + } + /// Recursively add nodes to execution stack. /// @param NodeImpl Node to schedule. /// @param Schedule Execution ordering to add node to. @@ -303,7 +323,7 @@ class node_impl { return false; if ((MCGType == sycl::detail::CG::CGTYPE::Kernel) && - (MCGType == sycl::detail::CG::CGTYPE::Kernel)) { + (Node->MCGType == sycl::detail::CG::CGTYPE::Kernel)) { sycl::detail::CGExecKernel *ExecKernelA = static_cast(MCommandGroup.get()); sycl::detail::CGExecKernel *ExecKernelB = @@ -348,6 +368,32 @@ class node_impl { return NumberOfNodes; } + /// Duplicate recursively a node and its successors + /// @param NodesMaps map that associates old nodes pointer to their + /// duplicates. This map is populated by this function. + /// @return a shared_ptr to the new duplicated node + std::shared_ptr duplicateNodeAndSuccessors( + std::map> &NodesMaps) { + // If the node has already been copied, we skip it + if (NodesMaps.find(this) != NodesMaps.end()) + return NodesMaps[this]; + + std::shared_ptr CpyNode; + if (MCGType == sycl::detail::CG::None) { + CpyNode = std::make_shared(); + CpyNode->MCGType = sycl::detail::CG::None; + } else { + CpyNode = std::make_shared(MCGType, getCGCopy()); + } + NodesMaps.insert({this, CpyNode}); + for (auto &NextNode : MSuccessors) { + auto Successor = NextNode->duplicateNodeAndSuccessors(NodesMaps); + + CpyNode->registerSuccessor(Successor, CpyNode); + } + return CpyNode; + } + private: /// Creates a copy of the node's CG by casting to it's actual type, then using /// that to copy construct and create a new unique ptr from that copy. @@ -400,6 +446,13 @@ class graph_impl { ~graph_impl(); + /// Copy constructor + graph_impl(const graph_impl &GraphImpl) + : MContext(GraphImpl.MContext), MDevice(GraphImpl.MDevice), + MRecordingQueues(), MEventsMap(), MInorderQueueMap() { + duplicateGraph(GraphImpl); + } + /// Remove node from list of root nodes. /// @param Root Node to remove from list of root nodes. void removeRoot(const std::shared_ptr &Root); @@ -491,6 +544,13 @@ class graph_impl { std::shared_ptr addSubgraphNodes(const std::list> &NodeList); + /// Duplicates and Adds sub-graph nodes from an executable graph to this + /// graph. + /// @param NodeList List of nodes from sub-graph in schedule order. + /// @return An empty node is used to schedule dependencies on this sub-graph. + std::shared_ptr + DuplicateAndAddSubgraphNodes(const std::shared_ptr &SubGraph); + /// Query for the context tied to this graph. /// @return Context associated with graph. sycl::context getContext() const { return MContext; } @@ -670,6 +730,12 @@ class graph_impl { /// @param Root Node to add to list of root nodes. void addRoot(const std::shared_ptr &Root); + /// Duplicates the nodes of GraphImpl and their dependencies to the caller + /// graph It also populates MEventsMap with the pointers to new duplicated + /// nodes. + /// @param GraphImpl Modifiable Graph to duplicate + void duplicateGraph(const graph_impl &GraphImpl); + /// Context associated with this graph. sycl::context MContext; /// Device associated with this graph. All graph nodes will execute on this @@ -754,6 +820,10 @@ class exec_graph_impl { return MSchedule; } + /// Query the graph_impl. + /// @return pointer to the graph_impl MGraphImpl . + const std::shared_ptr &getGraphImpl() const { return MGraphImpl; } + /// Prints the contents of the graph to a text file in DOT format /// @param GraphName is a string appended to the output file name void printGraphAsDot(const std::string GraphName) { diff --git a/sycl/source/handler.cpp b/sycl/source/handler.cpp index 1181947c70cd6..154d1b34c4cc9 100644 --- a/sycl/source/handler.cpp +++ b/sycl/source/handler.cpp @@ -1367,7 +1367,10 @@ void handler::ext_oneapi_graph( } // Store the node representing the subgraph in the handler so that we can // return it to the user later. - MSubgraphNode = ParentGraph->addSubgraphNodes(GraphImpl->getSchedule()); + // The nodes of the subgraph are duplicated when added to its parents. + // This avoids changing properties of the graph added as a subgraph. + MSubgraphNode = + ParentGraph->DuplicateAndAddSubgraphNodes(GraphImpl->getGraphImpl()); // If we are recording an in-order queue remember the subgraph node, so it // can be used as a dependency for any more nodes recorded from this queue. diff --git a/sycl/test-e2e/Graph/Explicit/sub_graph_multiple_submission.cpp b/sycl/test-e2e/Graph/Explicit/sub_graph_multiple_submission.cpp index 14e447c04104f..b8863b57c7290 100644 --- a/sycl/test-e2e/Graph/Explicit/sub_graph_multiple_submission.cpp +++ b/sycl/test-e2e/Graph/Explicit/sub_graph_multiple_submission.cpp @@ -6,9 +6,6 @@ // // CHECK-NOT: LEAK -// XFAIL:* -// Submit a graph as a subgraph more than once doesn't yet work. - #define GRAPH_E2E_EXPLICIT #include "../Inputs/sub_graph_multiple_submission.cpp" diff --git a/sycl/test-e2e/Graph/Explicit/sub_graph_two_parent_graphs.cpp b/sycl/test-e2e/Graph/Explicit/sub_graph_two_parent_graphs.cpp index cbf768203ec5a..4254a861fe344 100644 --- a/sycl/test-e2e/Graph/Explicit/sub_graph_two_parent_graphs.cpp +++ b/sycl/test-e2e/Graph/Explicit/sub_graph_two_parent_graphs.cpp @@ -6,9 +6,6 @@ // // CHECK-NOT: LEAK -// XFAIL: * -// Subgraph doesn't work properly in second parent graph - #define GRAPH_E2E_EXPLICIT #include "../Inputs/sub_graph_two_parent_graphs.cpp" diff --git a/sycl/test-e2e/Graph/Inputs/sub_graph_multiple_submission.cpp b/sycl/test-e2e/Graph/Inputs/sub_graph_multiple_submission.cpp index 81ad495da5268..a3a3f86b04895 100644 --- a/sycl/test-e2e/Graph/Inputs/sub_graph_multiple_submission.cpp +++ b/sycl/test-e2e/Graph/Inputs/sub_graph_multiple_submission.cpp @@ -62,7 +62,7 @@ int main() { Queue.memcpy(Output.data(), X, N * sizeof(float), E).wait(); for (size_t i = 0; i < N; i++) { - assert(Output[i] == -6.25f); + assert(Output[i] == -4.5f); } sycl::free(X, Queue); diff --git a/sycl/test-e2e/Graph/Inputs/sub_graph_two_parent_graphs.cpp b/sycl/test-e2e/Graph/Inputs/sub_graph_two_parent_graphs.cpp index 5c9732f4cc509..91ced1c25a9b0 100644 --- a/sycl/test-e2e/Graph/Inputs/sub_graph_two_parent_graphs.cpp +++ b/sycl/test-e2e/Graph/Inputs/sub_graph_two_parent_graphs.cpp @@ -50,7 +50,7 @@ int main() { auto ExecGraphA = GraphA.finalize(); auto B1 = add_node(GraphB, Queue, [&](handler &CGH) { - CGH.parallel_for(N, [=](id<1> it) { X[it] = static_cast(i); }); + CGH.parallel_for(N, [=](id<1> it) { X[it] = static_cast(it); }); }); auto B2 = add_node( diff --git a/sycl/test-e2e/Graph/RecordReplay/sub_graph_multiple_submission.cpp b/sycl/test-e2e/Graph/RecordReplay/sub_graph_multiple_submission.cpp index b8f4bfa3b3b83..aedd9e252e252 100644 --- a/sycl/test-e2e/Graph/RecordReplay/sub_graph_multiple_submission.cpp +++ b/sycl/test-e2e/Graph/RecordReplay/sub_graph_multiple_submission.cpp @@ -6,9 +6,6 @@ // // CHECK-NOT: LEAK -// XFAIL:* -// Submit a graph as a subgraph more than once doesn't yet work. - #define GRAPH_E2E_RECORD_REPLAY #include "../Inputs/sub_graph_multiple_submission.cpp" diff --git a/sycl/test-e2e/Graph/RecordReplay/sub_graph_two_parent_graphs.cpp b/sycl/test-e2e/Graph/RecordReplay/sub_graph_two_parent_graphs.cpp index 2d2eb17cd7078..6eb49c39583da 100644 --- a/sycl/test-e2e/Graph/RecordReplay/sub_graph_two_parent_graphs.cpp +++ b/sycl/test-e2e/Graph/RecordReplay/sub_graph_two_parent_graphs.cpp @@ -6,9 +6,6 @@ // // CHECK-NOT: LEAK -// XFAIL: * -// Subgraph doesn't work properly in second parent graph - #define GRAPH_E2E_RECORD_REPLAY #include "../Inputs/sub_graph_two_parent_graphs.cpp" diff --git a/sycl/test-e2e/Graph/RecordReplay/subgraph_interleaved_submit.cpp b/sycl/test-e2e/Graph/RecordReplay/subgraph_interleaved_submit.cpp deleted file mode 100644 index cb0c10aba3d40..0000000000000 --- a/sycl/test-e2e/Graph/RecordReplay/subgraph_interleaved_submit.cpp +++ /dev/null @@ -1,91 +0,0 @@ -// REQUIRES: level_zero, gpu -// RUN: %{build} -o %t.out -// RUN: %{run} %t.out -// RUN: %if ext_oneapi_level_zero %{env ZE_DEBUG=4 %{run} %t.out 2>&1 | FileCheck %s %} -// -// CHECK-NOT: LEAK - -// XFAIL:* -// Submit a graph as a subgraph more than once doesn't yet work. - -// Tests creating a parent graph with the same sub-graph interleaved with -// other nodes. -// The second run is to check that there are no leaks reported with the embedded -// ZE_DEBUG=4 testing capability. - -#include "../graph_common.hpp" - -int main() { - queue Queue{gpu_selector_v}; - - exp_ext::command_graph Graph{Queue.get_context(), Queue.get_device()}; - exp_ext::command_graph SubGraph{Queue.get_context(), Queue.get_device()}; - - const size_t N = 10; - float *X = malloc_device(N, Queue); - - SubGraph.begin_recording(Queue); - - auto S1 = Queue.submit([&](handler &CGH) { - CGH.parallel_for(N, [=](id<1> it) { - const size_t i = it[0]; - X[i] *= 2.0f; - }); - }); - - Queue.submit([&](handler &CGH) { - CGH.depends_on(S1); - CGH.parallel_for(N, [=](id<1> it) { - const size_t i = it[0]; - X[i] += 0.5f; - }); - }); - - SubGraph.end_recording(Queue); - - auto ExecSubGraph = SubGraph.finalize(); - - Graph.begin_recording(Queue); - - auto P1 = Queue.submit([&](handler &CGH) { - CGH.parallel_for(N, [=](id<1> it) { - const size_t i = it[0]; - X[i] = 1.0f; - }); - }); - - auto P2 = Queue.submit([&](handler &CGH) { - CGH.depends_on(P1); - CGH.ext_oneapi_graph(ExecSubGraph); - }); - - auto P3 = Queue.submit([&](handler &CGH) { - CGH.depends_on(P2); - CGH.parallel_for(range<1>{N}, [=](id<1> it) { - const size_t i = it[0]; - X[i] *= -1.0f; - }); - }); - - Queue.submit([&](handler &CGH) { - CGH.depends_on(P3); - CGH.ext_oneapi_graph(ExecSubGraph); - }); - - Graph.end_recording(); - - auto ExecGraph = Graph.finalize(); - - auto E = Queue.submit([&](handler &CGH) { CGH.ext_oneapi_graph(ExecGraph); }); - - std::vector Output(N); - Queue.memcpy(Output.data(), X, N * sizeof(float), E).wait(); - - for (size_t i = 0; i < N; i++) { - assert(Output[i] == -6.25f); - } - - sycl::free(X, Queue); - - return 0; -} diff --git a/sycl/test-e2e/Graph/RecordReplay/subgraph_two_parent_graphs.cpp b/sycl/test-e2e/Graph/RecordReplay/subgraph_two_parent_graphs.cpp deleted file mode 100644 index 5e2abe0dbf0f9..0000000000000 --- a/sycl/test-e2e/Graph/RecordReplay/subgraph_two_parent_graphs.cpp +++ /dev/null @@ -1,128 +0,0 @@ -// REQUIRES: level_zero, gpu -// RUN: %{build} -o %t.out -// RUN: %{run} %t.out -// RUN: %if ext_oneapi_level_zero %{env ZE_DEBUG=4 %{run} %t.out 2>&1 | FileCheck %s %} -// -// CHECK-NOT: LEAK - -// XFAIL: * -// Subgraph doesn't work properly in second parent graph - -// Tests adding an executable graph object as a sub-graph of two different -// parent graphs. -// The second run is to check that there are no leaks reported with the embedded -// ZE_DEBUG=4 testing capability. - -#include "../graph_common.hpp" - -int main() { - queue Queue{gpu_selector_v}; - - exp_ext::command_graph GraphA{Queue.get_context(), Queue.get_device()}; - exp_ext::command_graph GraphB{Queue.get_context(), Queue.get_device()}; - exp_ext::command_graph SubGraph{Queue.get_context(), Queue.get_device()}; - - const size_t N = 10; - float *X = malloc_device(N, Queue); - - SubGraph.begin_recording(Queue); - - auto S1 = Queue.submit([&](handler &CGH) { - CGH.parallel_for(N, [=](id<1> it) { - const size_t i = it[0]; - X[i] *= 2.0f; - }); - }); - - Queue.submit([&](handler &CGH) { - CGH.depends_on(S1); - CGH.parallel_for(N, [=](id<1> it) { - const size_t i = it[0]; - X[i] += 0.5f; - }); - }); - - SubGraph.end_recording(Queue); - - auto ExecSubGraph = SubGraph.finalize(); - - GraphA.begin_recording(Queue); - - auto A1 = Queue.submit([&](handler &CGH) { - CGH.parallel_for(N, [=](id<1> it) { - const size_t i = it[0]; - X[i] = 1.0f; - }); - }); - - auto A2 = Queue.submit([&](handler &CGH) { - CGH.depends_on(A1); - CGH.ext_oneapi_graph(ExecSubGraph); - }); - - Queue.submit([&](handler &CGH) { - CGH.depends_on(A2); - CGH.parallel_for(range<1>{N}, [=](id<1> it) { - const size_t i = it[0]; - X[i] *= -1.0f; - }); - }); - - GraphA.end_recording(); - - auto ExecGraphA = GraphA.finalize(); - - GraphB.begin_recording(Queue); - - auto B1 = Queue.submit([&](handler &CGH) { - CGH.parallel_for(N, [=](id<1> it) { - const size_t i = it[0]; - X[i] = static_cast(i); - }); - }); - - auto B2 = Queue.submit([&](handler &CGH) { - CGH.depends_on(B1); - CGH.ext_oneapi_graph(ExecSubGraph); - }); - - Queue.submit([&](handler &CGH) { - CGH.depends_on(B2); - CGH.parallel_for(range<1>{N}, [=](id<1> it) { - const size_t i = it[0]; - X[i] *= X[i]; - }); - }); - - GraphB.end_recording(); - auto ExecGraphB = GraphB.finalize(); - - auto EventA1 = - Queue.submit([&](handler &CGH) { CGH.ext_oneapi_graph(ExecGraphA); }); - std::vector OutputA(N); - auto EventA2 = Queue.memcpy(OutputA.data(), X, N * sizeof(float), EventA1); - - auto EventB1 = Queue.submit([&](handler &CGH) { - CGH.depends_on(EventA2); - CGH.ext_oneapi_graph(ExecGraphB); - }); - std::vector OutputB(N); - Queue.memcpy(OutputB.data(), X, N * sizeof(float), EventB1); - Queue.wait(); - - auto refB = [](size_t i) { - float result = static_cast(i); - result = result * 2.0f + 0.5f; - result *= result; - return result; - }; - - for (size_t i = 0; i < N; i++) { - assert(OutputA[i] == -2.5f); - assert(OutputB[i] == refB(i)); - } - - sycl::free(X, Queue); - - return 0; -} diff --git a/sycl/unittests/Extensions/CommandGraph.cpp b/sycl/unittests/Extensions/CommandGraph.cpp index 6c8e7ce2dd3e5..a2b3dcd16bd5a 100644 --- a/sycl/unittests/Extensions/CommandGraph.cpp +++ b/sycl/unittests/Extensions/CommandGraph.cpp @@ -548,7 +548,6 @@ bool checkExecGraphSchedule( } return true; } - } // anonymous namespace class CommandGraphTest : public ::testing::Test { @@ -822,8 +821,11 @@ TEST_F(CommandGraphTest, SubGraph) { ASSERT_EQ(sycl::detail::getSyclObjImpl(MainGraph)->MRoots.size(), 1lu); ASSERT_EQ(sycl::detail::getSyclObjImpl(Node1MainGraph)->MSuccessors.size(), 1lu); - ASSERT_EQ(sycl::detail::getSyclObjImpl(Node1MainGraph)->MSuccessors.front(), - sycl::detail::getSyclObjImpl(Node1Graph)); + // Subgraph nodes are duplicated when inserted to parent graph. + // we thus check the node type only. + ASSERT_TRUE( + *(sycl::detail::getSyclObjImpl(Node1MainGraph)->MSuccessors.front()) == + *(sycl::detail::getSyclObjImpl(Node1Graph))); ASSERT_EQ(sycl::detail::getSyclObjImpl(Node2MainGraph)->MSuccessors.size(), 1lu); ASSERT_EQ(sycl::detail::getSyclObjImpl(Node1MainGraph)->MPredecessors.size(), @@ -839,9 +841,9 @@ TEST_F(CommandGraphTest, SubGraph) { ASSERT_EQ(Schedule.size(), 4ul); ASSERT_EQ(*ScheduleIt, sycl::detail::getSyclObjImpl(Node1MainGraph)); ScheduleIt++; - ASSERT_EQ(*ScheduleIt, sycl::detail::getSyclObjImpl(Node1Graph)); + ASSERT_TRUE(*(*ScheduleIt) == *(sycl::detail::getSyclObjImpl(Node1Graph))); ScheduleIt++; - ASSERT_EQ(*ScheduleIt, sycl::detail::getSyclObjImpl(Node2Graph)); + ASSERT_TRUE(*(*ScheduleIt) == *(sycl::detail::getSyclObjImpl(Node2Graph))); ScheduleIt++; ASSERT_EQ(*ScheduleIt, sycl::detail::getSyclObjImpl(Node3MainGraph)); ASSERT_EQ(Queue.get_context(), MainGraphExecImpl->getContext()); @@ -882,8 +884,8 @@ TEST_F(CommandGraphTest, RecordSubGraph) { ASSERT_EQ(Schedule.size(), 4ul); // The first and fourth nodes should have events associated with MainGraph but - // not graph. The second and third nodes were added as a sub-graph and should - // have events associated with Graph but not MainGraph. + // not graph. The second and third nodes were added as a sub-graph and + // duplicated. They should not have events associated with Graph or MainGraph. ASSERT_ANY_THROW( sycl::detail::getSyclObjImpl(Graph)->getEventForNode(*ScheduleIt)); ASSERT_EQ( @@ -893,14 +895,14 @@ TEST_F(CommandGraphTest, RecordSubGraph) { ScheduleIt++; ASSERT_ANY_THROW( sycl::detail::getSyclObjImpl(MainGraph)->getEventForNode(*ScheduleIt)); - ASSERT_EQ(sycl::detail::getSyclObjImpl(Graph)->getEventForNode(*ScheduleIt), - sycl::detail::getSyclObjImpl(Node1Graph)); + ASSERT_ANY_THROW( + sycl::detail::getSyclObjImpl(Graph)->getEventForNode(*ScheduleIt)); ScheduleIt++; ASSERT_ANY_THROW( sycl::detail::getSyclObjImpl(MainGraph)->getEventForNode(*ScheduleIt)); - ASSERT_EQ(sycl::detail::getSyclObjImpl(Graph)->getEventForNode(*ScheduleIt), - sycl::detail::getSyclObjImpl(Node2Graph)); + ASSERT_ANY_THROW( + sycl::detail::getSyclObjImpl(Graph)->getEventForNode(*ScheduleIt)); ScheduleIt++; ASSERT_ANY_THROW( @@ -1774,3 +1776,15 @@ TEST_F(CommandGraphTest, InvalidHostAccessor) { // Graph is now out of scope so we should be able to create a host_accessor ASSERT_NO_THROW({ host_accessor HostAcc{Buffer}; }); } + +TEST_F(CommandGraphTest, CopyConstructor) { + // Create graph with multiple roots + addKernels(Graph); + addKernels(Graph); + addKernels(Graph); + + auto GraphImpl = sycl::detail::getSyclObjImpl(Graph); + auto GraphCopyImpl(*GraphImpl); + ASSERT_EQ(GraphCopyImpl.hasSimilarStructure(GraphImpl), true); + ASSERT_NE(*(GraphImpl->MRoots.begin()), *(GraphCopyImpl.MRoots.begin())); +} From def1366a0978caa4b5b5f7a5cb284b175d73c80f Mon Sep 17 00:00:00 2001 From: Maxime France-Pillois Date: Wed, 9 Aug 2023 12:23:13 +0100 Subject: [PATCH 2/6] [SYCL][Graph] Duplicate sub-graph nodes Code factorization. Function renaming. Removes the duplication of in-recording queue when duplicating graphs. --- sycl/source/detail/graph_impl.cpp | 6 +----- sycl/source/detail/graph_impl.hpp | 18 ++++-------------- sycl/source/handler.cpp | 2 +- 3 files changed, 6 insertions(+), 20 deletions(-) diff --git a/sycl/source/detail/graph_impl.cpp b/sycl/source/detail/graph_impl.cpp index d37a1ac933a63..9207de35d38ea 100644 --- a/sycl/source/detail/graph_impl.cpp +++ b/sycl/source/detail/graph_impl.cpp @@ -163,7 +163,7 @@ std::shared_ptr graph_impl::addSubgraphNodes( return this->add(Outputs); } -std::shared_ptr graph_impl::DuplicateAndAddSubgraphNodes( +std::shared_ptr graph_impl::duplicateAndAddSubgraphNodes( const std::shared_ptr &SubGraph) { std::map> NodesMaps; std::vector> SubGraphRoots; @@ -429,10 +429,6 @@ void graph_impl::duplicateGraph(const graph_impl &GraphImpl) { this->addRoot(CpyRoot); } - for (auto &Queue : GraphImpl.MRecordingQueues) { - MRecordingQueues.insert(Queue); - } - for (auto &Element : GraphImpl.MEventsMap) { if (NodesMaps.find(Element.second.get()) != NodesMaps.end()) addEventForNode(Element.first, NodesMaps[Element.second.get()]); diff --git a/sycl/source/detail/graph_impl.hpp b/sycl/source/detail/graph_impl.hpp index cf0054a529f55..e511a673caaaf 100644 --- a/sycl/source/detail/graph_impl.hpp +++ b/sycl/source/detail/graph_impl.hpp @@ -313,26 +313,16 @@ class node_impl { /// Tests is the caller is similar to Node /// @return True if the two nodes are similars bool isSimilar(std::shared_ptr Node) { - if (MCGType != Node->MCGType) - return false; - if (MSuccessors.size() != Node->MSuccessors.size()) return false; if (MPredecessors.size() != Node->MPredecessors.size()) return false; - if ((MCGType == sycl::detail::CG::CGTYPE::Kernel) && - (Node->MCGType == sycl::detail::CG::CGTYPE::Kernel)) { - sycl::detail::CGExecKernel *ExecKernelA = - static_cast(MCommandGroup.get()); - sycl::detail::CGExecKernel *ExecKernelB = - static_cast(Node->MCommandGroup.get()); + if (*this == *Node.get()) + return true; - if (ExecKernelA->MKernelName.compare(ExecKernelB->MKernelName) != 0) - return false; - } - return true; + return false; } /// Recursive traversal of successor nodes checking for @@ -549,7 +539,7 @@ class graph_impl { /// @param NodeList List of nodes from sub-graph in schedule order. /// @return An empty node is used to schedule dependencies on this sub-graph. std::shared_ptr - DuplicateAndAddSubgraphNodes(const std::shared_ptr &SubGraph); + duplicateAndAddSubgraphNodes(const std::shared_ptr &SubGraph); /// Query for the context tied to this graph. /// @return Context associated with graph. diff --git a/sycl/source/handler.cpp b/sycl/source/handler.cpp index 154d1b34c4cc9..17fdd94d992e0 100644 --- a/sycl/source/handler.cpp +++ b/sycl/source/handler.cpp @@ -1370,7 +1370,7 @@ void handler::ext_oneapi_graph( // The nodes of the subgraph are duplicated when added to its parents. // This avoids changing properties of the graph added as a subgraph. MSubgraphNode = - ParentGraph->DuplicateAndAddSubgraphNodes(GraphImpl->getGraphImpl()); + ParentGraph->duplicateAndAddSubgraphNodes(GraphImpl->getGraphImpl()); // If we are recording an in-order queue remember the subgraph node, so it // can be used as a dependency for any more nodes recorded from this queue. From 59fa03eabab14d1247f37dc8a9d0393e774871f6 Mon Sep 17 00:00:00 2001 From: Maxime France-Pillois Date: Fri, 11 Aug 2023 12:43:44 +0100 Subject: [PATCH 3/6] [SYCL][Graph] Duplicate sub-graph nodes Changes the duplication strategy for subgraph. Duplication is now based on scheduled node list. --- sycl/source/detail/graph_impl.cpp | 63 +++++++++++++------- sycl/source/detail/graph_impl.hpp | 68 ++++++++++++++++------ sycl/source/handler.cpp | 3 +- sycl/unittests/Extensions/CommandGraph.cpp | 2 +- 4 files changed, 96 insertions(+), 40 deletions(-) diff --git a/sycl/source/detail/graph_impl.cpp b/sycl/source/detail/graph_impl.cpp index 9207de35d38ea..a15fa82b2c640 100644 --- a/sycl/source/detail/graph_impl.cpp +++ b/sycl/source/detail/graph_impl.cpp @@ -123,6 +123,16 @@ void appendExitNodesFromRoot(std::shared_ptr CurrentNode, } } +void duplicateNode(const std::shared_ptr Node, + std::shared_ptr &NodeCopy) { + if (Node->MCGType == sycl::detail::CG::None) { + NodeCopy = std::make_shared(); + NodeCopy->MCGType = sycl::detail::CG::None; + } else { + NodeCopy = std::make_shared(Node->MCGType, Node->getCGCopy()); + } +} + } // anonymous namespace void exec_graph_impl::schedule() { @@ -164,24 +174,37 @@ std::shared_ptr graph_impl::addSubgraphNodes( } std::shared_ptr graph_impl::duplicateAndAddSubgraphNodes( - const std::shared_ptr &SubGraph) { - std::map> NodesMaps; - std::vector> SubGraphRoots; - std::vector> SubGraphOutputs; - - for (auto &Root : SubGraph->MRoots) { - auto NewRoot = Root->duplicateNodeAndSuccessors(NodesMaps); - SubGraphRoots.push_back(NewRoot); - appendExitNodesFromRoot(NewRoot, SubGraphOutputs); + const std::list> &NodeList) { + std::map, std::shared_ptr> NodesMap; + std::list> NewNodeList; + + for (std::list>::const_iterator NodeIt = + NodeList.end(); + NodeIt != NodeList.begin();) { + --NodeIt; + auto Node = *NodeIt; + std::shared_ptr NodeCopy; + duplicateNode(Node, NodeCopy); + NewNodeList.push_back(NodeCopy); + NodesMap.insert({Node, NodeCopy}); + for (auto &NextNode : Node->MSuccessors) { + if (NodesMap.find(NextNode) != NodesMap.end()) { + auto Successor = NodesMap[NextNode]; + NodeCopy->registerSuccessor(Successor, NodeCopy); + } + } } - // Recursively walk the graph to find exit nodes and connect up the inputs - // TODO: Consider caching exit nodes so we don't have to do this - for (auto NodeImpl : MRoots) { - connectToExitNodes(NodeImpl, SubGraphRoots); - } + return addSubgraphNodes(NewNodeList); +} - return this->add(SubGraphOutputs); +std::shared_ptr graph_impl::addSubgraphNodes( + const std::shared_ptr &SubGraphExec, + const bool DuplicateNodes) { + if (DuplicateNodes) { + return duplicateAndAddSubgraphNodes(SubGraphExec->getSchedule()); + } + return addSubgraphNodes(SubGraphExec->getSchedule()); } void graph_impl::addRoot(const std::shared_ptr &Root) { @@ -423,15 +446,15 @@ void graph_impl::makeEdge(std::shared_ptr Src, } void graph_impl::duplicateGraph(const graph_impl &GraphImpl) { - std::map> NodesMaps; + std::map> NodesMap; for (auto &Root : GraphImpl.MRoots) { - auto CpyRoot = Root->duplicateNodeAndSuccessors(NodesMaps); - this->addRoot(CpyRoot); + auto RootCopy = Root->duplicateNodeAndSuccessors(NodesMap); + this->addRoot(RootCopy); } for (auto &Element : GraphImpl.MEventsMap) { - if (NodesMaps.find(Element.second.get()) != NodesMaps.end()) - addEventForNode(Element.first, NodesMaps[Element.second.get()]); + if (NodesMap.find(Element.second.get()) != NodesMap.end()) + addEventForNode(Element.first, NodesMap[Element.second.get()]); } } diff --git a/sycl/source/detail/graph_impl.hpp b/sycl/source/detail/graph_impl.hpp index e511a673caaaf..c981014f8d35b 100644 --- a/sycl/source/detail/graph_impl.hpp +++ b/sycl/source/detail/graph_impl.hpp @@ -99,8 +99,7 @@ class node_impl { if (MCGType != Node.MCGType) return false; - if ((MCGType == sycl::detail::CG::CGTYPE::Kernel) && - (Node.MCGType == sycl::detail::CG::CGTYPE::Kernel)) { + if (MCGType == sycl::detail::CG::CGTYPE::Kernel) { sycl::detail::CGExecKernel *ExecKernelA = static_cast(MCommandGroup.get()); sycl::detail::CGExecKernel *ExecKernelB = @@ -109,6 +108,27 @@ class node_impl { if (ExecKernelA->MKernelName.compare(ExecKernelB->MKernelName) != 0) return false; } + if (MCGType == sycl::detail::CG::CGTYPE::CopyUSM) { + sycl::detail::CGCopyUSM *CopyA = + static_cast(MCommandGroup.get()); + sycl::detail::CGCopyUSM *CopyB = + static_cast(MCommandGroup.get()); + if ((CopyA->getSrc() != CopyB->getSrc()) || + (CopyA->getDst() != CopyB->getDst()) || + (CopyA->getLength() == CopyB->getLength())) + return false; + } + if ((MCGType == sycl::detail::CG::CGTYPE::CopyAccToAcc) || + (MCGType == sycl::detail::CG::CGTYPE::CopyAccToPtr) || + (MCGType == sycl::detail::CG::CGTYPE::CopyPtrToAcc)) { + sycl::detail::CGCopy *CopyA = + static_cast(MCommandGroup.get()); + sycl::detail::CGCopy *CopyB = + static_cast(MCommandGroup.get()); + if ((CopyA->getSrc() != CopyB->getSrc()) || + (CopyA->getDst() != CopyB->getDst())) + return false; + } return true; } @@ -368,20 +388,20 @@ class node_impl { if (NodesMaps.find(this) != NodesMaps.end()) return NodesMaps[this]; - std::shared_ptr CpyNode; + std::shared_ptr NodeCopy; if (MCGType == sycl::detail::CG::None) { - CpyNode = std::make_shared(); - CpyNode->MCGType = sycl::detail::CG::None; + NodeCopy = std::make_shared(); + NodeCopy->MCGType = sycl::detail::CG::None; } else { - CpyNode = std::make_shared(MCGType, getCGCopy()); + NodeCopy = std::make_shared(MCGType, getCGCopy()); } - NodesMaps.insert({this, CpyNode}); + NodesMaps.insert({this, NodeCopy}); for (auto &NextNode : MSuccessors) { auto Successor = NextNode->duplicateNodeAndSuccessors(NodesMaps); - CpyNode->registerSuccessor(Successor, CpyNode); + NodeCopy->registerSuccessor(Successor, NodeCopy); } - return CpyNode; + return NodeCopy; } private: @@ -528,18 +548,15 @@ class graph_impl { "No event has been recorded for the specified graph node"); } - /// Adds sub-graph nodes from an executable graph to this graph. - /// @param NodeList List of nodes from sub-graph in schedule order. - /// @return An empty node is used to schedule dependencies on this sub-graph. - std::shared_ptr - addSubgraphNodes(const std::list> &NodeList); - /// Duplicates and Adds sub-graph nodes from an executable graph to this /// graph. - /// @param NodeList List of nodes from sub-graph in schedule order. + /// @param SubGraphExec sub-graph to add to the parent. + /// @param DuplicateNodes if true nodes are duplicated first, then add the the + /// parent graph /// @return An empty node is used to schedule dependencies on this sub-graph. std::shared_ptr - duplicateAndAddSubgraphNodes(const std::shared_ptr &SubGraph); + addSubgraphNodes(const std::shared_ptr &SubGraphExec, + const bool DuplicateNodes = true); /// Query for the context tied to this graph. /// @return Context associated with graph. @@ -759,6 +776,23 @@ class graph_impl { /// Controls whether we allow buffers to be used in the graph. Set by the /// presence of the assume_buffer_outlives_graph property. bool MAllowBuffers = false; + + /// Insert node into list of root nodes. + /// @param Root Node to add to list of root nodes. + void addRoot(const std::shared_ptr &Root); + + /// Adds sub-graph nodes from an executable graph to this graph. + /// @param NodeList List of nodes from sub-graph in schedule order. + /// @return An empty node is used to schedule dependencies on this sub-graph. + std::shared_ptr + addSubgraphNodes(const std::list> &NodeList); + + /// Duplicates and Adds sub-graph nodes from an executable graph to this + /// graph. + /// @param NodeList List of nodes from sub-graph in schedule order. + /// @return An empty node is used to schedule dependencies on this sub-graph. + std::shared_ptr duplicateAndAddSubgraphNodes( + const std::list> &NodeList); }; /// Class representing the implementation of command_graph. diff --git a/sycl/source/handler.cpp b/sycl/source/handler.cpp index 17fdd94d992e0..a31b9ded69dfd 100644 --- a/sycl/source/handler.cpp +++ b/sycl/source/handler.cpp @@ -1369,8 +1369,7 @@ void handler::ext_oneapi_graph( // return it to the user later. // The nodes of the subgraph are duplicated when added to its parents. // This avoids changing properties of the graph added as a subgraph. - MSubgraphNode = - ParentGraph->duplicateAndAddSubgraphNodes(GraphImpl->getGraphImpl()); + MSubgraphNode = ParentGraph->addSubgraphNodes(GraphImpl); // If we are recording an in-order queue remember the subgraph node, so it // can be used as a dependency for any more nodes recorded from this queue. diff --git a/sycl/unittests/Extensions/CommandGraph.cpp b/sycl/unittests/Extensions/CommandGraph.cpp index a2b3dcd16bd5a..3599cb8812a17 100644 --- a/sycl/unittests/Extensions/CommandGraph.cpp +++ b/sycl/unittests/Extensions/CommandGraph.cpp @@ -822,7 +822,7 @@ TEST_F(CommandGraphTest, SubGraph) { ASSERT_EQ(sycl::detail::getSyclObjImpl(Node1MainGraph)->MSuccessors.size(), 1lu); // Subgraph nodes are duplicated when inserted to parent graph. - // we thus check the node type only. + // we thus check the node content only. ASSERT_TRUE( *(sycl::detail::getSyclObjImpl(Node1MainGraph)->MSuccessors.front()) == *(sycl::detail::getSyclObjImpl(Node1Graph))); From 9188dd11518094ce1071a0841797127a6bd98d8c Mon Sep 17 00:00:00 2001 From: Maxime France-Pillois Date: Fri, 11 Aug 2023 15:00:03 +0100 Subject: [PATCH 4/6] [SYCL][Graph] Duplicate sub-graph nodes Removes graph_impl copy constructor --- sycl/source/detail/graph_impl.cpp | 13 ---------- sycl/source/detail/graph_impl.hpp | 43 ------------------------------- 2 files changed, 56 deletions(-) diff --git a/sycl/source/detail/graph_impl.cpp b/sycl/source/detail/graph_impl.cpp index a15fa82b2c640..920aa1ad582ac 100644 --- a/sycl/source/detail/graph_impl.cpp +++ b/sycl/source/detail/graph_impl.cpp @@ -445,19 +445,6 @@ void graph_impl::makeEdge(std::shared_ptr Src, removeRoot(Dest); // remove receiver from root node list } -void graph_impl::duplicateGraph(const graph_impl &GraphImpl) { - std::map> NodesMap; - for (auto &Root : GraphImpl.MRoots) { - auto RootCopy = Root->duplicateNodeAndSuccessors(NodesMap); - this->addRoot(RootCopy); - } - - for (auto &Element : GraphImpl.MEventsMap) { - if (NodesMap.find(Element.second.get()) != NodesMap.end()) - addEventForNode(Element.first, NodesMap[Element.second.get()]); - } -} - // Check if nodes are empty and if so loop back through predecessors until we // find the real dependency. void exec_graph_impl::findRealDeps( diff --git a/sycl/source/detail/graph_impl.hpp b/sycl/source/detail/graph_impl.hpp index c981014f8d35b..15a57e7a35a43 100644 --- a/sycl/source/detail/graph_impl.hpp +++ b/sycl/source/detail/graph_impl.hpp @@ -378,32 +378,6 @@ class node_impl { return NumberOfNodes; } - /// Duplicate recursively a node and its successors - /// @param NodesMaps map that associates old nodes pointer to their - /// duplicates. This map is populated by this function. - /// @return a shared_ptr to the new duplicated node - std::shared_ptr duplicateNodeAndSuccessors( - std::map> &NodesMaps) { - // If the node has already been copied, we skip it - if (NodesMaps.find(this) != NodesMaps.end()) - return NodesMaps[this]; - - std::shared_ptr NodeCopy; - if (MCGType == sycl::detail::CG::None) { - NodeCopy = std::make_shared(); - NodeCopy->MCGType = sycl::detail::CG::None; - } else { - NodeCopy = std::make_shared(MCGType, getCGCopy()); - } - NodesMaps.insert({this, NodeCopy}); - for (auto &NextNode : MSuccessors) { - auto Successor = NextNode->duplicateNodeAndSuccessors(NodesMaps); - - NodeCopy->registerSuccessor(Successor, NodeCopy); - } - return NodeCopy; - } - private: /// Creates a copy of the node's CG by casting to it's actual type, then using /// that to copy construct and create a new unique ptr from that copy. @@ -456,13 +430,6 @@ class graph_impl { ~graph_impl(); - /// Copy constructor - graph_impl(const graph_impl &GraphImpl) - : MContext(GraphImpl.MContext), MDevice(GraphImpl.MDevice), - MRecordingQueues(), MEventsMap(), MInorderQueueMap() { - duplicateGraph(GraphImpl); - } - /// Remove node from list of root nodes. /// @param Root Node to remove from list of root nodes. void removeRoot(const std::shared_ptr &Root); @@ -733,16 +700,6 @@ class graph_impl { /// @return True if a cycle is detected, false if not. bool checkForCycles(); - /// Insert node into list of root nodes. - /// @param Root Node to add to list of root nodes. - void addRoot(const std::shared_ptr &Root); - - /// Duplicates the nodes of GraphImpl and their dependencies to the caller - /// graph It also populates MEventsMap with the pointers to new duplicated - /// nodes. - /// @param GraphImpl Modifiable Graph to duplicate - void duplicateGraph(const graph_impl &GraphImpl); - /// Context associated with this graph. sycl::context MContext; /// Device associated with this graph. All graph nodes will execute on this From 2a641629d299f7c1389c9e1a069941b88e844d5f Mon Sep 17 00:00:00 2001 From: Maxime France-Pillois Date: Mon, 14 Aug 2023 09:53:51 +0100 Subject: [PATCH 5/6] [SYCL][Graph] Duplicate sub-graph nodes Corrects typos --- sycl/source/detail/graph_impl.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sycl/source/detail/graph_impl.hpp b/sycl/source/detail/graph_impl.hpp index 15a57e7a35a43..40dac84823ff7 100644 --- a/sycl/source/detail/graph_impl.hpp +++ b/sycl/source/detail/graph_impl.hpp @@ -518,8 +518,8 @@ class graph_impl { /// Duplicates and Adds sub-graph nodes from an executable graph to this /// graph. /// @param SubGraphExec sub-graph to add to the parent. - /// @param DuplicateNodes if true nodes are duplicated first, then add the the - /// parent graph + /// @param DuplicateNodes if true nodes are duplicated first, then added to + /// the parent graph /// @return An empty node is used to schedule dependencies on this sub-graph. std::shared_ptr addSubgraphNodes(const std::shared_ptr &SubGraphExec, From 82f2c313c7cf8792f57f5d96c9e55ab3ebc88ee7 Mon Sep 17 00:00:00 2001 From: Maxime France-Pillois Date: Mon, 14 Aug 2023 11:07:42 +0100 Subject: [PATCH 6/6] [SYCL][Graph] Duplicate sub-graph nodes Rebase fixes + function/code refactorization --- sycl/source/detail/graph_impl.cpp | 19 ++++++------------- sycl/source/detail/graph_impl.hpp | 16 +++------------- sycl/unittests/Extensions/CommandGraph.cpp | 12 ------------ 3 files changed, 9 insertions(+), 38 deletions(-) diff --git a/sycl/source/detail/graph_impl.cpp b/sycl/source/detail/graph_impl.cpp index 920aa1ad582ac..7d26e61f9c4d8 100644 --- a/sycl/source/detail/graph_impl.cpp +++ b/sycl/source/detail/graph_impl.cpp @@ -150,7 +150,7 @@ graph_impl::~graph_impl() { } } -std::shared_ptr graph_impl::addSubgraphNodes( +std::shared_ptr graph_impl::addNodesToExits( const std::list> &NodeList) { // Find all input and output nodes from the node list std::vector> Inputs; @@ -173,11 +173,13 @@ std::shared_ptr graph_impl::addSubgraphNodes( return this->add(Outputs); } -std::shared_ptr graph_impl::duplicateAndAddSubgraphNodes( - const std::list> &NodeList) { +std::shared_ptr graph_impl::addSubgraphNodes( + const std::shared_ptr &SubGraphExec) { std::map, std::shared_ptr> NodesMap; std::list> NewNodeList; + std::list> NodeList = SubGraphExec->getSchedule(); + for (std::list>::const_iterator NodeIt = NodeList.end(); NodeIt != NodeList.begin();) { @@ -195,16 +197,7 @@ std::shared_ptr graph_impl::duplicateAndAddSubgraphNodes( } } - return addSubgraphNodes(NewNodeList); -} - -std::shared_ptr graph_impl::addSubgraphNodes( - const std::shared_ptr &SubGraphExec, - const bool DuplicateNodes) { - if (DuplicateNodes) { - return duplicateAndAddSubgraphNodes(SubGraphExec->getSchedule()); - } - return addSubgraphNodes(SubGraphExec->getSchedule()); + return addNodesToExits(NewNodeList); } void graph_impl::addRoot(const std::shared_ptr &Root) { diff --git a/sycl/source/detail/graph_impl.hpp b/sycl/source/detail/graph_impl.hpp index 40dac84823ff7..7b961e78d85af 100644 --- a/sycl/source/detail/graph_impl.hpp +++ b/sycl/source/detail/graph_impl.hpp @@ -518,12 +518,9 @@ class graph_impl { /// Duplicates and Adds sub-graph nodes from an executable graph to this /// graph. /// @param SubGraphExec sub-graph to add to the parent. - /// @param DuplicateNodes if true nodes are duplicated first, then added to - /// the parent graph /// @return An empty node is used to schedule dependencies on this sub-graph. std::shared_ptr - addSubgraphNodes(const std::shared_ptr &SubGraphExec, - const bool DuplicateNodes = true); + addSubgraphNodes(const std::shared_ptr &SubGraphExec); /// Query for the context tied to this graph. /// @return Context associated with graph. @@ -738,18 +735,11 @@ class graph_impl { /// @param Root Node to add to list of root nodes. void addRoot(const std::shared_ptr &Root); - /// Adds sub-graph nodes from an executable graph to this graph. + /// Adds nodes to the exit nodes of this graph. /// @param NodeList List of nodes from sub-graph in schedule order. /// @return An empty node is used to schedule dependencies on this sub-graph. std::shared_ptr - addSubgraphNodes(const std::list> &NodeList); - - /// Duplicates and Adds sub-graph nodes from an executable graph to this - /// graph. - /// @param NodeList List of nodes from sub-graph in schedule order. - /// @return An empty node is used to schedule dependencies on this sub-graph. - std::shared_ptr duplicateAndAddSubgraphNodes( - const std::list> &NodeList); + addNodesToExits(const std::list> &NodeList); }; /// Class representing the implementation of command_graph. diff --git a/sycl/unittests/Extensions/CommandGraph.cpp b/sycl/unittests/Extensions/CommandGraph.cpp index 3599cb8812a17..f5ad662f20c53 100644 --- a/sycl/unittests/Extensions/CommandGraph.cpp +++ b/sycl/unittests/Extensions/CommandGraph.cpp @@ -1776,15 +1776,3 @@ TEST_F(CommandGraphTest, InvalidHostAccessor) { // Graph is now out of scope so we should be able to create a host_accessor ASSERT_NO_THROW({ host_accessor HostAcc{Buffer}; }); } - -TEST_F(CommandGraphTest, CopyConstructor) { - // Create graph with multiple roots - addKernels(Graph); - addKernels(Graph); - addKernels(Graph); - - auto GraphImpl = sycl::detail::getSyclObjImpl(Graph); - auto GraphCopyImpl(*GraphImpl); - ASSERT_EQ(GraphCopyImpl.hasSimilarStructure(GraphImpl), true); - ASSERT_NE(*(GraphImpl->MRoots.begin()), *(GraphCopyImpl.MRoots.begin())); -}