Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix adding empty nodes to the graph results in two root nodes #237

Merged
merged 5 commits into from
Jun 26, 2023
Merged

Fix adding empty nodes to the graph results in two root nodes #237

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
940526b
Add empty node during graph recording only
julianmi Jun 20, 2023
53efe8b
Add missing move statement
julianmi Jun 21, 2023
8e0c1a8
Explicitly mark CG::None as empty nodes
julianmi Jun 21, 2023
f55deae
Apply comments from code review
julianmi Jun 22, 2023
fa9991c
Fix Doxygen comment
julianmi Jun 26, 2023
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: 0 additions & 3 deletions sycl/source/detail/graph_impl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,6 @@ graph_impl::add(const std::shared_ptr<graph_impl> &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);
}

Expand Down
8 changes: 6 additions & 2 deletions sycl/source/detail/graph_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Bensuo marked this conversation as resolved.
Show resolved Hide resolved

/// Get a deep copy of this node's command group
/// @return A unique ptr to the new command group object.
Expand Down Expand Up @@ -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.
julianmi marked this conversation as resolved.
Show resolved Hide resolved
void createURCommandBuffers(sycl::device Device);
void createURCommandBuffers(sycl::device Device);

/// Query for the context tied to this graph.
/// @return Context associated with graph.
Expand Down
6 changes: 3 additions & 3 deletions sycl/source/handler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,11 +383,11 @@ event handler::finalize() {
std::shared_ptr<ext::oneapi::experimental::detail::graph_impl> 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;
}

if (GraphImpl) {
auto EventImpl = std::make_shared<detail::event_impl>();
Bensuo marked this conversation as resolved.
Show resolved Hide resolved

// Extract relevant data from the handler and pass to graph to create a
Expand Down