Skip to content

Commit

Permalink
[SYCL][Graph] Makes command graph functions thread-safe (#265)
Browse files Browse the repository at this point in the history
* [SYCL][Graph] Makes command graph functions thread-safe

Addresses comments made on the first PR commit.
Mutexes are now added to Graph implementation entry points
instead of end points as was the case in the previous commit.
Adds "build_pthread_inc" lit test macro to facilitate the
compilation of the threading tests.
Removes std::barrier (std-20) dependency in threading tests.

Addresses Issue: #85

* [SYCL][Graph] Makes command graph functions thread-safe

Moves threading tests that do not require a device to run to unitests

* Update sycl/source/detail/graph_impl.cpp

Co-authored-by: Ben Tracy <ben.tracy@codeplay.com>

* [SYCL][Graph] Makes command graph functions thread-safe

Adds some comments.

* Update sycl/source/handler.cpp

Co-authored-by: Pablo Reble <pablo.reble@intel.com>

* Update sycl/source/detail/graph_impl.hpp

Co-authored-by: Ewan Crawford <ewan@codeplay.com>

* [SYCL][Graph] Makes command graph functions thread-safe

Adds dedidacted sub-class to unitests for multi-threading unitests

* [SYCL][Graph] Makes command graph functions thread-safe

Adds comments

* [SYCL][Graph] thread-safe: bug fix after rebase

---------

Co-authored-by: Ben Tracy <ben.tracy@codeplay.com>
Co-authored-by: Pablo Reble <pablo.reble@intel.com>
Co-authored-by: Ewan Crawford <ewan@codeplay.com>
  • Loading branch information
4 people committed Aug 10, 2023
1 parent dc70ead commit 3c968a0
Show file tree
Hide file tree
Showing 13 changed files with 686 additions and 23 deletions.
18 changes: 16 additions & 2 deletions sycl/source/detail/graph_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@ graph_impl::add(sycl::detail::CG::CGTYPE CGType,
checkForRequirement(Req, NodePtr, UniqueDeps);
}
}

// Add any nodes specified by event dependencies into the dependency list
for (auto Dep : CommandGroup->getEvents()) {
if (auto NodeImpl = MEventsMap.find(Dep); NodeImpl != MEventsMap.end()) {
Expand Down Expand Up @@ -343,6 +342,8 @@ void exec_graph_impl::createCommandBuffers(sycl::device Device) {
}

exec_graph_impl::~exec_graph_impl() {
WriteLock Lock(MMutex);

// clear all recording queue if not done before (no call to end_recording)
MGraphImpl->clearQueues();

Expand All @@ -368,6 +369,8 @@ exec_graph_impl::~exec_graph_impl() {
sycl::event
exec_graph_impl::enqueue(const std::shared_ptr<sycl::detail::queue_impl> &Queue,
sycl::detail::CG::StorageInitHelper CGData) {
WriteLock Lock(MMutex);

auto CreateNewEvent([&]() {
auto NewEvent = std::make_shared<sycl::detail::event_impl>(Queue);
NewEvent->setContextImpl(Queue->getContextImplPtr());
Expand Down Expand Up @@ -472,6 +475,7 @@ node modifiable_command_graph::addImpl(const std::vector<node> &Deps) {
DepImpls.push_back(sycl::detail::getSyclObjImpl(D));
}

graph_impl::WriteLock Lock(impl->MMutex);
std::shared_ptr<detail::node_impl> NodeImpl = impl->add(DepImpls);
return sycl::detail::createSyclObjFromImpl<node>(NodeImpl);
}
Expand All @@ -483,6 +487,7 @@ node modifiable_command_graph::addImpl(std::function<void(handler &)> CGF,
DepImpls.push_back(sycl::detail::getSyclObjImpl(D));
}

graph_impl::WriteLock Lock(impl->MMutex);
std::shared_ptr<detail::node_impl> NodeImpl =
impl->add(impl, CGF, {}, DepImpls);
return sycl::detail::createSyclObjFromImpl<node>(NodeImpl);
Expand All @@ -494,6 +499,7 @@ void modifiable_command_graph::make_edge(node &Src, node &Dest) {
std::shared_ptr<detail::node_impl> ReceiverImpl =
sycl::detail::getSyclObjImpl(Dest);

graph_impl::WriteLock Lock(impl->MMutex);
SenderImpl->registerSuccessor(ReceiverImpl,
SenderImpl); // register successor
impl->removeRoot(ReceiverImpl); // remove receiver from root node list
Expand Down Expand Up @@ -521,6 +527,7 @@ bool modifiable_command_graph::begin_recording(queue &RecordingQueue) {

if (QueueImpl->getCommandGraph() == nullptr) {
QueueImpl->setCommandGraph(impl);
graph_impl::WriteLock Lock(impl->MMutex);
impl->addQueue(QueueImpl);
return true;
}
Expand All @@ -542,12 +549,16 @@ bool modifiable_command_graph::begin_recording(
return QueueStateChanged;
}

bool modifiable_command_graph::end_recording() { return impl->clearQueues(); }
bool modifiable_command_graph::end_recording() {
graph_impl::WriteLock Lock(impl->MMutex);
return impl->clearQueues();
}

bool modifiable_command_graph::end_recording(queue &RecordingQueue) {
auto QueueImpl = sycl::detail::getSyclObjImpl(RecordingQueue);
if (QueueImpl->getCommandGraph() == impl) {
QueueImpl->setCommandGraph(nullptr);
graph_impl::WriteLock Lock(impl->MMutex);
impl->removeQueue(QueueImpl);
return true;
}
Expand All @@ -574,6 +585,9 @@ executable_command_graph::executable_command_graph(
const std::shared_ptr<detail::graph_impl> &Graph, const sycl::context &Ctx)
: MTag(rand()),
impl(std::make_shared<detail::exec_graph_impl>(Ctx, Graph)) {
// Graph is read and written in this scope so we lock
// this graph with full priviledges.
graph_impl::WriteLock Lock(Graph->MMutex);
finalizeImpl(); // Create backend representation for executable graph
}

Expand Down
Loading

0 comments on commit 3c968a0

Please sign in to comment.