[SYCL][Graph] Throw exception when explicit add called on a graph recording a queue (#283)

Factorizes the exception throwing method when the explicit API is used on a graph recording a queue
Improves the test while_recording to test throwing an invalid exception for the two explicit graph:add entry points.

Addresses issue #271

* Update sycl/source/detail/graph_impl.hpp

Co-authored-by: Ben Tracy <[email protected]>

* Update sycl/test-e2e/Graph/Explicit/while_recording.cpp

Co-authored-by: Ben Tracy <[email protected]>

* Update sycl/source/detail/graph_impl.hpp

Co-authored-by: Ewan Crawford <[email protected]>

---------

Co-authored-by: Ben Tracy <[email protected]>
Co-authored-by: Ewan Crawford <[email protected]>

Author: 3 people

sycl/source/detail/graph_impl.cpp

@@ -305,11 +305,7 @@ bool graph_impl::checkForCycles() {
 
 void graph_impl::makeEdge(std::shared_ptr<node_impl> Src,
                           std::shared_ptr<node_impl> Dest) {
-  if (MRecordingQueues.size()) {
-    throw sycl::exception(make_error_code(sycl::errc::invalid),
-                          "make_edge() cannot be called when a queue is "
-                          "currently recording commands to a graph.");
-  }
+  throwIfGraphRecordingQueue("make_edge()");
   if (Src == Dest) {
     throw sycl::exception(
         make_error_code(sycl::errc::invalid),
@@ -610,6 +606,7 @@ modifiable_command_graph::modifiable_command_graph(
                                                 PropList)) {}
 
 node modifiable_command_graph::addImpl(const std::vector<node> &Deps) {
+  impl->throwIfGraphRecordingQueue("Explicit API \"Add()\" function");
   std::vector<std::shared_ptr<detail::node_impl>> DepImpls;
   for (auto &D : Deps) {
     DepImpls.push_back(sycl::detail::getSyclObjImpl(D));
@@ -621,6 +618,7 @@ node modifiable_command_graph::addImpl(const std::vector<node> &Deps) {
 
 node modifiable_command_graph::addImpl(std::function<void(handler &)> CGF,
                                        const std::vector<node> &Deps) {
+  impl->throwIfGraphRecordingQueue("Explicit API \"Add()\" function");
   std::vector<std::shared_ptr<detail::node_impl>> DepImpls;
   for (auto &D : Deps) {
     DepImpls.push_back(sycl::detail::getSyclObjImpl(D));

sycl/source/detail/graph_impl.hpp

@@ -343,6 +343,18 @@ class graph_impl {
   void makeEdge(std::shared_ptr<node_impl> Src,
                 std::shared_ptr<node_impl> Dest);
 
+  /// Throws an invalid exception if this function is called
+  /// while a queue is recording commands to the graph.
+  /// @param ExceptionMsg Message to append to the exception message
+  void throwIfGraphRecordingQueue(const std::string ExceptionMsg) const {
+    if (MRecordingQueues.size()) {
+      throw sycl::exception(make_error_code(sycl::errc::invalid),
+                            ExceptionMsg +
+                                " cannot be called when a queue "
+                                "is currently recording commands to a graph.");
+    }
+  }
+
 private:
   /// Iterate over the graph depth-first and run \p NodeFunc on each node.
   /// @param NodeFunc A function which receives as input a node in the graph to

sycl/test-e2e/Graph/Explicit/add_node_while_recording.cpp

@@ -6,9 +6,6 @@
 //
 // CHECK-NOT: LEAK
 
-// Expected Fail as exception not implemented yet
-// XFAIL: *
-
 // Tests attempting to add a node to a command_graph while it is being
 // recorded to by a queue is an error.
 
@@ -30,8 +27,17 @@ int main() {
       Success = true;
     }
   }
+  assert(Success);
 
-  Graph.end_recording();
+  Success = false;
+  try {
+    Graph.add({});
+  } catch (sycl::exception &E) {
+    auto StdErrc = E.code().value();
+    Success = (StdErrc == static_cast<int>(errc::invalid));
+  }
   assert(Success);
+
+  Graph.end_recording();
   return 0;
 }