From 940526bd911533313a420e9ec13f7c4a57d1d1eb Mon Sep 17 00:00:00 2001 From: Julian Miller Date: Tue, 20 Jun 2023 17:05:28 +0200 Subject: [PATCH 1/5] Add empty node during graph recording only --- sycl/source/detail/graph_impl.cpp | 3 --- sycl/source/handler.cpp | 5 ++--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/sycl/source/detail/graph_impl.cpp b/sycl/source/detail/graph_impl.cpp index 75b0566c8dbcc..4bf4b7d531cf4 100644 --- a/sycl/source/detail/graph_impl.cpp +++ b/sycl/source/detail/graph_impl.cpp @@ -146,9 +146,6 @@ graph_impl::add(const std::shared_ptr &Impl, if (Handler.MSubgraphNode) { return Handler.MSubgraphNode; } - if (Handler.MCGType == sycl::detail::CG::None) { - return this->add(Dep); - } return this->add(Handler.MCGType, std::move(Handler.MGraphNodeCG), Dep); } diff --git a/sycl/source/handler.cpp b/sycl/source/handler.cpp index 4c75372372890..ad8a61690aacf 100644 --- a/sycl/source/handler.cpp +++ b/sycl/source/handler.cpp @@ -383,11 +383,10 @@ event handler::finalize() { std::shared_ptr GraphImpl; if (MGraph) { GraphImpl = MGraph; + CommandGroup.reset( + new detail::CG(detail::CG::None, std::move(CGData), MCodeLoc)); } else if (auto QueueGraph = MQueue->getCommandGraph(); QueueGraph) { GraphImpl = QueueGraph; - } - - if (GraphImpl) { auto EventImpl = std::make_shared(); // Extract relevant data from the handler and pass to graph to create a From 53efe8b502905d4c54853df29b61982d2675e6eb Mon Sep 17 00:00:00 2001 From: Julian Miller Date: Wed, 21 Jun 2023 14:34:30 +0200 Subject: [PATCH 2/5] Add missing move statement --- sycl/source/handler.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/sycl/source/handler.cpp b/sycl/source/handler.cpp index ad8a61690aacf..2a2c61001079b 100644 --- a/sycl/source/handler.cpp +++ b/sycl/source/handler.cpp @@ -385,6 +385,7 @@ event handler::finalize() { GraphImpl = MGraph; CommandGroup.reset( new detail::CG(detail::CG::None, std::move(CGData), MCodeLoc)); + MGraphNodeCG = std::move(CommandGroup); } else if (auto QueueGraph = MQueue->getCommandGraph(); QueueGraph) { GraphImpl = QueueGraph; auto EventImpl = std::make_shared(); From 8e0c1a8652a4af3d53c90d62f2958af507a4b1c8 Mon Sep 17 00:00:00 2001 From: Julian Miller Date: Wed, 21 Jun 2023 18:13:27 +0200 Subject: [PATCH 3/5] Explicitly mark CG::None as empty nodes --- sycl/source/detail/graph_impl.hpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/sycl/source/detail/graph_impl.hpp b/sycl/source/detail/graph_impl.hpp index 651d82a1ca30c..b234416d4482f 100644 --- a/sycl/source/detail/graph_impl.hpp +++ b/sycl/source/detail/graph_impl.hpp @@ -106,7 +106,11 @@ class node_impl { /// Query if this is an empty node. /// @return True if this is an empty node, false otherwise. - bool isEmpty() const { return MIsEmpty; } + bool isEmpty() const { + if (MCGType == sycl::detail::CG::None) + return true; + return MIsEmpty; + } /// Get a deep copy of this node's command group /// @return A unique ptr to the new command group object. @@ -367,7 +371,7 @@ class exec_graph_impl { /// Turns the internal graph representation into UR command-buffers for a /// device. /// @param D Device to create backend command-buffers for. - void createURCommandBuffers(sycl::device Device); + void createURCommandBuffers(sycl::device Device); /// Query for the context tied to this graph. /// @return Context associated with graph. From f55deaebb661ce9af740e453678ff2e34c1eea8f Mon Sep 17 00:00:00 2001 From: Julian Miller Date: Thu, 22 Jun 2023 18:27:22 +0200 Subject: [PATCH 4/5] Apply comments from code review --- sycl/source/detail/graph_impl.hpp | 11 ++--------- sycl/source/handler.cpp | 7 ++----- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/sycl/source/detail/graph_impl.hpp b/sycl/source/detail/graph_impl.hpp index b234416d4482f..be41c15b9d213 100644 --- a/sycl/source/detail/graph_impl.hpp +++ b/sycl/source/detail/graph_impl.hpp @@ -43,9 +43,6 @@ class node_impl { /// Command group object which stores all args etc needed to enqueue the node std::unique_ptr MCommandGroup; - /// True if an empty node, false otherwise. - bool MIsEmpty = false; - /// Add successor to the node. /// @param Node Node to add as a successor. /// @param Prev Predecessor to \p node being added as successor. @@ -65,7 +62,7 @@ class node_impl { } /// Construct an empty node. - node_impl() : MIsEmpty(true) {} + node_impl() {} /// Construct a node representing a command-group. /// @param CGType Type of the command-group. @@ -106,11 +103,7 @@ class node_impl { /// Query if this is an empty node. /// @return True if this is an empty node, false otherwise. - bool isEmpty() const { - if (MCGType == sycl::detail::CG::None) - return true; - return MIsEmpty; - } + bool isEmpty() const { return MCGType == sycl::detail::CG::None; } /// Get a deep copy of this node's command group /// @return A unique ptr to the new command group object. diff --git a/sycl/source/handler.cpp b/sycl/source/handler.cpp index 2a2c61001079b..3c633e232a502 100644 --- a/sycl/source/handler.cpp +++ b/sycl/source/handler.cpp @@ -380,23 +380,20 @@ event handler::finalize() { if (detail::pi::trace(detail::pi::TraceLevel::PI_TRACE_ALL)) { std::cout << "WARNING: An empty command group is submitted." << std::endl; } - std::shared_ptr GraphImpl; if (MGraph) { - GraphImpl = MGraph; CommandGroup.reset( new detail::CG(detail::CG::None, std::move(CGData), MCodeLoc)); MGraphNodeCG = std::move(CommandGroup); } else if (auto QueueGraph = MQueue->getCommandGraph(); QueueGraph) { - GraphImpl = QueueGraph; auto EventImpl = std::make_shared(); // Extract relevant data from the handler and pass to graph to create a // new node representing this command group. std::shared_ptr NodeImpl = - GraphImpl->add(CGData.MEvents); + QueueGraph->add(CGData.MEvents); // Associate an event with this new node and return the event. - GraphImpl->addEventForNode(EventImpl, NodeImpl); + QueueGraph->addEventForNode(EventImpl, NodeImpl); return detail::createSyclObjFromImpl(EventImpl); } From fa9991cc6bf924f3f1bcd2612c7deddab94dbbd8 Mon Sep 17 00:00:00 2001 From: Julian Miller Date: Mon, 26 Jun 2023 15:33:36 +0200 Subject: [PATCH 5/5] Fix Doxygen comment Co-authored-by: Ewan Crawford --- sycl/source/detail/graph_impl.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sycl/source/detail/graph_impl.hpp b/sycl/source/detail/graph_impl.hpp index be41c15b9d213..4fa9b326bbfd6 100644 --- a/sycl/source/detail/graph_impl.hpp +++ b/sycl/source/detail/graph_impl.hpp @@ -363,7 +363,7 @@ class exec_graph_impl { /// Turns the internal graph representation into UR command-buffers for a /// device. - /// @param D Device to create backend command-buffers for. + /// @param Device Device to create backend command-buffers for. void createURCommandBuffers(sycl::device Device); /// Query for the context tied to this graph.