[SYCL][Graph] Make calling begin_recording repeatedly an error (#15192)

Since future features may add more properties/state to queue recording
it is now an error to call `begin_recording()` on the same queue more
than once, rather than a no-op. This is particularly relevant in
scenarios where a recording queue is shared between multiple threads.
Applications are now encouraged to manage queue state from a single
thread rather than freely call `begin/end_recording()` from any thread.

- Calling begin_recording on a recording queue is now always an error.
- Update spec, implementation and unit tests to reflect changes.

Author: Bensuo

sycl/doc/extensions/experimental/sycl_ext_oneapi_graph.asciidoc

@@ -1284,7 +1284,7 @@ begin_recording(queue& recordingQueue,
 ----
 
 |Synchronously changes the state of `recordingQueue` to the
-`queue_state::recording` state. This operation is a no-op if `recordingQueue`
+`queue_state::recording` state. This operation is an error if `recordingQueue`
 is already in the `queue_state::recording` state.
 
 Parameters:
@@ -1299,7 +1299,7 @@ Parameters:
 Exceptions:
 
 * Throws synchronously with error code `invalid` if `recordingQueue` is
-  already recording to a different graph.
+  already recording to a graph.
 
 * Throws synchronously with error code `invalid` if `recordingQueue` is
   associated with a device or context that is different from the device
@@ -1314,7 +1314,7 @@ begin_recording(const std::vector<queue>& recordingQueues,
 ----
 
 |Synchronously changes the state of each queue in `recordingQueues` to the
-`queue_state::recording` state. This operation is a no-op for any queue in
+`queue_state::recording` state. This operation is an error for any queue in
 `recordingQueues` that is already in the `queue_state::recording` state.
 
 Parameters:
@@ -1328,8 +1328,8 @@ Parameters:
 
 Exceptions:
 
-* Throws synchronously with error code `invalid` if the any queue in
-  `recordingQueues` is already recording to a different graph.
+* Throws synchronously with error code `invalid` if any queue in
+  `recordingQueues` is already recording to a graph.
 
 * Throws synchronously with error code `invalid` if any of `recordingQueues`
   is associated with a device or context that is different from the device

sycl/source/detail/graph_impl.cpp

@@ -1586,6 +1586,14 @@ void modifiable_command_graph::begin_recording(
 
   auto QueueImpl = sycl::detail::getSyclObjImpl(RecordingQueue);
   assert(QueueImpl);
+
+  auto QueueGraph = QueueImpl->getCommandGraph();
+  if (QueueGraph != nullptr) {
+    throw sycl::exception(sycl::make_error_code(errc::invalid),
+                          "begin_recording cannot be called for a queue which "
+                          "is already in the recording state.");
+  }
+
   if (QueueImpl->get_context() != impl->getContext()) {
     throw sycl::exception(sycl::make_error_code(errc::invalid),
                           "begin_recording called for a queue whose context "
@@ -1603,13 +1611,6 @@ void modifiable_command_graph::begin_recording(
                           "can NOT be recorded.");
   }
 
-  auto QueueGraph = QueueImpl->getCommandGraph();
-  if (QueueGraph != nullptr && QueueGraph != impl) {
-    throw sycl::exception(sycl::make_error_code(errc::invalid),
-                          "begin_recording called for a queue which is already "
-                          "recording to a different graph.");
-  }
-
   impl->beginRecording(QueueImpl);
 }
 

sycl/unittests/Extensions/CommandGraph/CommandGraph.cpp

@@ -131,19 +131,20 @@ TEST_F(CommandGraphTest, BeginEndRecording) {
   sycl::queue Queue2{Queue.get_context(), Dev};
 
   // Test throwing behaviour
-  // Check we can repeatedly begin recording on the same queues
+  // Check that repeatedly calling begin recording on the same queues is an
+  // error
   ASSERT_NO_THROW(Graph.begin_recording(Queue));
-  ASSERT_NO_THROW(Graph.begin_recording(Queue));
-  ASSERT_NO_THROW(Graph.begin_recording(Queue2));
+  ASSERT_ANY_THROW(Graph.begin_recording(Queue));
   ASSERT_NO_THROW(Graph.begin_recording(Queue2));
+  ASSERT_ANY_THROW(Graph.begin_recording(Queue2));
   // Check we can repeatedly end recording on the same queues
   ASSERT_NO_THROW(Graph.end_recording(Queue));
   ASSERT_NO_THROW(Graph.end_recording(Queue));
   ASSERT_NO_THROW(Graph.end_recording(Queue2));
   ASSERT_NO_THROW(Graph.end_recording(Queue2));
   // Vector versions
   ASSERT_NO_THROW(Graph.begin_recording({Queue, Queue2}));
-  ASSERT_NO_THROW(Graph.begin_recording({Queue, Queue2}));
+  ASSERT_ANY_THROW(Graph.begin_recording({Queue, Queue2}));
   ASSERT_NO_THROW(Graph.end_recording({Queue, Queue2}));
   ASSERT_NO_THROW(Graph.end_recording({Queue, Queue2}));