[SYCL][Graph] Update begin_recording and end_recording

sycl/doc/extensions/experimental/sycl_ext_oneapi_graph.asciidoc

@@ -618,12 +618,12 @@ public:
  command_graph<graph_state::executable>
  finalize(const property_list& propList = {}) const;
 
- bool begin_recording(queue& recordingQueue, const property_list& propList = {});
- bool begin_recording(const std::vector<queue>& recordingQueues, const property_list& propList = {});
+ void begin_recording(queue& recordingQueue, const property_list& propList = {});
+ void begin_recording(const std::vector<queue>& recordingQueues, const property_list& propList = {});
 
- bool end_recording();
- bool end_recording(queue& recordingQueue);
- bool end_recording(const std::vector<queue>& recordingQueues);
+ void end_recording();
+ void end_recording(queue& recordingQueue);
+ void end_recording(const std::vector<queue>& recordingQueues);
 
  node add(const property_list& propList = {});
 
@@ -1148,13 +1148,14 @@ queue recording.
 |
 [source, c++]
 ----
-bool
+void
 begin_recording(queue& recordingQueue,
  const property_list& propList = {})
 ----
 
 |Synchronously changes the state of `recordingQueue` to the
-`queue_state::recording` state.
+`queue_state::recording` state. This operation is a no-op if `recordingQueue`
+is already in the `queue_state::recording` state.
 
 Parameters:
 
@@ -1165,9 +1166,6 @@ Parameters:
 * `propList` - Optional parameter for passing properties. Properties for
  the `command_graph` class are defined in <<graph-properties, Graph Properties>>.
 
-Returns: `true` if `recordingQueue` has its state changed from
-`queue_state::executing` to `queue_state::recording`, `false` otherwise.
-
 Exceptions:
 
 * Throws synchronously with error code `invalid` if `recordingQueue` is
@@ -1180,13 +1178,13 @@ Exceptions:
 |
 [source, c++]
 ----
-bool
+void
 begin_recording(const std::vector<queue>& recordingQueues,
  const property_list& propList = {})
 ----
 
 |Synchronously changes the state of each queue in `recordingQueues` to the
-`queue_state::recording` state.
+`queue_state::recording` state. This operation is a no-op for any queue in `recordingQueues` that is already in the `queue_state::recording` state.
 
 Parameters:
 
@@ -1197,9 +1195,6 @@ Parameters:
 * `propList` - Optional parameter for passing properties. Properties for
  the `command_graph` class are defined in <<graph-properties, Graph Properties>>.
 
-Returns: `true` if any queue in `recordingQueues` has its state changed from
-`queue_state::executing` to `queue_state::recording`, `false` otherwise.
-
 Exceptions:
 
 * Throws synchronously with error code `invalid` if the any queue in
@@ -1212,31 +1207,28 @@ Exceptions:
 |
 [source, c++]
 ----
-bool end_recording()
+void end_recording()
 ----
 
 |Synchronously finishes recording on all queues that are recording to the
-graph and sets their state to `queue_state::executing`.
-
-Returns: `true` if any queue recording to the graph has its state changed from
-`queue_state::recording` to `queue_state::executing`, `false` otherwise.
+graph and sets their state to `queue_state::executing`. This operation is
+a no-op for any queue in the graph that is already in the
+`queue_state::executing` state.
 
 |
 [source, c++]
 ----
-bool end_recording(queue& recordingQueue)
+void end_recording(queue& recordingQueue)
 ----
 
 |Synchronously changes the state of `recordingQueue` to the
-`queue_state::executing` state.
+`queue_state::executing` state. This operation is a no-op if `recordingQueue`
+is already in the `queue_state::executing` state.
 
 Parameters:
 
 * `recordingQueue` - A `sycl::queue` object to change to the executing state.
 
-Returns: `true` if `recordingQueue` has its state changed from
-`queue_state::recording` to `queue_state::executing`, `false` otherwise.
-
 Exceptions:
 
 * Throws synchronously with error code `invalid` if `recordingQueue` is
@@ -1245,20 +1237,17 @@ Exceptions:
 |
 [source, c++]
 ----
-bool end_recording(const std::vector<queue>& recordingQueues)
+void end_recording(const std::vector<queue>& recordingQueues)
 ----
 
 |Synchronously changes the state of each queue in `recordingQueues` to the
-`queue_state::executing` state.
+`queue_state::executing` state. This operation is a no-op for any queue in `recordingQueues` that is already in the `queue_state::executing` state.
 
 Parameters:
 
 * `recordingQueues` - List of `sycl::queue` objects to change to the executing
  state.
 
-Returns: `true` if any queue in `recordingQueues` has its state changed from
-`queue_state::recording` to `queue_state::executing`, `false` otherwise.
-
 Exceptions:
 
 * Throws synchronously with error code `invalid` if any queue in
@@ -1677,13 +1666,6 @@ being thrown in the default queue executing state, will still be thrown when a
 queue is in the recording state. Queue query methods operate as usual in
 recording mode, as opposed to throwing.
 
-The `command_graph::begin_recording` and `command_graph::end_recording`
-entry-points return a `bool` value informing the user whether a related queue
-state change occurred. False is returned rather than throwing an exception when
-no queue state is changed. This design is because the queues are already in
-the state the user desires, so if the function threw an exception in this case,
-the application would likely swallow it and then proceed.
-
 === Interaction With Other Extensions [[extension-interaction]]
 
 This section defines the interaction of `sycl_ext_oneapi_graph` with other

sycl/include/sycl/ext/oneapi/experimental/graph.hpp

@@ -271,35 +271,29 @@ class __SYCL_EXPORT modifiable_command_graph {
  /// it.
  /// @param RecordingQueue The queue to change state on and associate this
  /// graph with.
- /// @return True if the queue had its state changed from executing to
- /// recording.
- bool begin_recording(queue &RecordingQueue);
+ /// @param PropList Property list used to pass properties for recording.
+ void begin_recording(queue &RecordingQueue,
+  const property_list &PropList = {});
 
  /// Change the state of multiple queues to be recording and associate this
  /// graph with each of them.
  /// @param RecordingQueues The queues to change state on and associate this
  /// graph with.
- /// @return True if any queue had its state changed from executing to
- /// recording.
- bool begin_recording(const std::vector<queue> &RecordingQueues);
+ /// @param PropList Property list used to pass properties for recording.
+ void begin_recording(const std::vector<queue> &RecordingQueues,
+  const property_list &PropList = {});
 
  /// Set all queues currently recording to this graph to the executing state.
- /// @return True if any queue had its state changed from recording to
- /// executing.
- bool end_recording();
+ void end_recording();
 
  /// Set a queue currently recording to this graph to the executing state.
  /// @param RecordingQueue The queue to change state on.
- /// @return True if the queue had its state changed from recording to
- /// executing.
- bool end_recording(queue &RecordingQueue);
+ void end_recording(queue &RecordingQueue);
 
  /// Set multiple queues currently recording to this graph to the executing
  /// state.
  /// @param RecordingQueues The queues to change state on.
- /// @return True if any queue had its state changed from recording to
- /// executing.
- bool end_recording(const std::vector<queue> &RecordingQueues);
+ void end_recording(const std::vector<queue> &RecordingQueues);
 
  /// Synchronous operation that writes a DOT formatted description of the graph
  /// to the provided path. By default, this includes the graph topology, node

sycl/source/detail/graph_impl.cpp

@@ -1516,7 +1516,10 @@ modifiable_command_graph::finalize(const sycl::property_list &PropList) const {
  this->impl, this->impl->getContext(), PropList};
 }
 
-bool modifiable_command_graph::begin_recording(queue &RecordingQueue) {
+void modifiable_command_graph::begin_recording(
+ queue &RecordingQueue, const sycl::property_list &PropList) {
+ std::ignore = PropList;
+
  auto QueueImpl = sycl::detail::getSyclObjImpl(RecordingQueue);
  assert(QueueImpl);
  if (QueueImpl->get_context() != impl->getContext()) {
@@ -1551,56 +1554,46 @@ bool modifiable_command_graph::begin_recording(queue &RecordingQueue) {
  QueueImpl->setCommandGraph(impl);
  graph_impl::WriteLock Lock(impl->MMutex);
  impl->addQueue(QueueImpl);
- return true;
  }
  if (QueueImpl->getCommandGraph() != impl) {
  throw sycl::exception(sycl::make_error_code(errc::invalid),
  "begin_recording called for a queue which is already "
  "recording to a different graph.");
  }
- // Queue was already recording to this graph.
- return false;
 }
 
-bool modifiable_command_graph::begin_recording(
- const std::vector<queue> &RecordingQueues) {
- bool QueueStateChanged = false;
+void modifiable_command_graph::begin_recording(
+ const std::vector<queue> &RecordingQueues,
+  const sycl::property_list &PropList) {
  for (queue Queue : RecordingQueues) {
- QueueStateChanged |= this->begin_recording(Queue);
+ this->begin_recording(Queue, PropList);
  }
- return QueueStateChanged;
 }
 
-bool modifiable_command_graph::end_recording() {
+void modifiable_command_graph::end_recording() {
  graph_impl::WriteLock Lock(impl->MMutex);
- return impl->clearQueues();
+ impl->clearQueues();
 }
 
-bool modifiable_command_graph::end_recording(queue &RecordingQueue) {
+void modifiable_command_graph::end_recording(queue &RecordingQueue) {
  auto QueueImpl = sycl::detail::getSyclObjImpl(RecordingQueue);
  if (QueueImpl && QueueImpl->getCommandGraph() == impl) {
  QueueImpl->setCommandGraph(nullptr);
  graph_impl::WriteLock Lock(impl->MMutex);
  impl->removeQueue(QueueImpl);
- return true;
  }
  if (QueueImpl->getCommandGraph() != nullptr) {
  throw sycl::exception(sycl::make_error_code(errc::invalid),
  "end_recording called for a queue which is recording "
  "to a different graph.");
  }
-
- // Queue was not recording to a graph.
- return false;
 }
 
-bool modifiable_command_graph::end_recording(
+void modifiable_command_graph::end_recording(
  const std::vector<queue> &RecordingQueues) {
- bool QueueStateChanged = false;
  for (queue Queue : RecordingQueues) {
- QueueStateChanged |= this->end_recording(Queue);
+ this->end_recording(Queue);
  }
- return QueueStateChanged;
 }
 
 void modifiable_command_graph::print_graph(std::string path,

sycl/test/abi/sycl_symbols_linux.dump

@@ -3078,8 +3078,8 @@ _ZN4sycl3_V13ext6oneapi12experimental6detail24executable_command_graphC2ERKSt10s
 _ZN4sycl3_V13ext6oneapi12experimental6detail24modifiable_command_graph13end_recordingERKSt6vectorINS0_5queueESaIS7_EE
 _ZN4sycl3_V13ext6oneapi12experimental6detail24modifiable_command_graph13end_recordingERNS0_5queueE
 _ZN4sycl3_V13ext6oneapi12experimental6detail24modifiable_command_graph13end_recordingEv
-_ZN4sycl3_V13ext6oneapi12experimental6detail24modifiable_command_graph15begin_recordingERKSt6vectorINS0_5queueESaIS7_EE
-_ZN4sycl3_V13ext6oneapi12experimental6detail24modifiable_command_graph15begin_recordingERNS0_5queueE
+_ZN4sycl3_V13ext6oneapi12experimental6detail24modifiable_command_graph15begin_recordingERKSt6vectorINS0_5queueESaIS7_EERKNS0_13property_listE
+_ZN4sycl3_V13ext6oneapi12experimental6detail24modifiable_command_graph15begin_recordingERNS0_5queueERKNS0_13property_listE
 _ZN4sycl3_V13ext6oneapi12experimental6detail24modifiable_command_graph24addGraphLeafDependenciesENS3_4nodeE
 _ZN4sycl3_V13ext6oneapi12experimental6detail24modifiable_command_graph7addImplERKSt6vectorINS3_4nodeESaIS7_EE
 _ZN4sycl3_V13ext6oneapi12experimental6detail24modifiable_command_graph7addImplESt8functionIFvRNS0_7handlerEEERKSt6vectorINS3_4nodeESaISC_EE
@@ -4093,14 +4093,14 @@ _ZNK4sycl3_V16device9getNativeEv
 _ZNK4sycl3_V16kernel11get_backendEv
 _ZNK4sycl3_V16kernel11get_contextEv
 _ZNK4sycl3_V16kernel13getNativeImplEv
-_ZNK4sycl3_V16kernel16get_backend_infoINS0_4info6device15backend_versionEEENS0_6detail20is_backend_info_descIT_E11return_typeEv
-_ZNK4sycl3_V16kernel16get_backend_infoINS0_4info6device7versionEEENS0_6detail20is_backend_info_descIT_E11return_typeEv
-_ZNK4sycl3_V16kernel16get_backend_infoINS0_4info8platform7versionEEENS0_6detail20is_backend_info_descIT_E11return_typeEv
 _ZNK4sycl3_V16kernel13get_info_implINS0_4info6kernel10attributesEEENS0_6detail11ABINeutralTINS6_19is_kernel_info_descIT_E11return_typeEE4typeEv
 _ZNK4sycl3_V16kernel13get_info_implINS0_4info6kernel13function_nameEEENS0_6detail11ABINeutralTINS6_19is_kernel_info_descIT_E11return_typeEE4typeEv
 _ZNK4sycl3_V16kernel13get_info_implINS0_4info6kernel15reference_countEEENS0_6detail11ABINeutralTINS6_19is_kernel_info_descIT_E11return_typeEE4typeEv
 _ZNK4sycl3_V16kernel13get_info_implINS0_4info6kernel7contextEEENS0_6detail11ABINeutralTINS6_19is_kernel_info_descIT_E11return_typeEE4typeEv
 _ZNK4sycl3_V16kernel13get_info_implINS0_4info6kernel8num_argsEEENS0_6detail11ABINeutralTINS6_19is_kernel_info_descIT_E11return_typeEE4typeEv
+_ZNK4sycl3_V16kernel16get_backend_infoINS0_4info6device15backend_versionEEENS0_6detail20is_backend_info_descIT_E11return_typeEv
+_ZNK4sycl3_V16kernel16get_backend_infoINS0_4info6device7versionEEENS0_6detail20is_backend_info_descIT_E11return_typeEv
+_ZNK4sycl3_V16kernel16get_backend_infoINS0_4info8platform7versionEEENS0_6detail20is_backend_info_descIT_E11return_typeEv
 _ZNK4sycl3_V16kernel17get_kernel_bundleEv
 _ZNK4sycl3_V16kernel19ext_oneapi_get_infoINS0_3ext6oneapi12experimental4info21kernel_queue_specific23max_num_work_group_syncEEENT_11return_typeERKNS0_5queueE
 _ZNK4sycl3_V16kernel3getEv

sycl/unittests/Extensions/CommandGraph/CommandGraph.cpp

@@ -155,48 +155,6 @@ TEST_F(CommandGraphTest, BeginEndRecording) {
  // Trying to end when it is recording to a different graph should throw
  ASSERT_ANY_THROW(Graph2.end_recording(Queue));
  Graph.end_recording(Queue);
-
- // Testing return values of begin and end recording
- // Queue should change state so should return true here
- ASSERT_TRUE(Graph.begin_recording(Queue));
- // But not changed state here
- ASSERT_FALSE(Graph.begin_recording(Queue));
-
- // Queue2 should change state so should return true here
- ASSERT_TRUE(Graph.begin_recording(Queue2));
- // But not changed state here
- ASSERT_FALSE(Graph.begin_recording(Queue2));
-
- // Queue should have changed state so should return true
- ASSERT_TRUE(Graph.end_recording(Queue));
- // But not changed state here
- ASSERT_FALSE(Graph.end_recording(Queue));
-
- // Should end recording on Queue2
- ASSERT_TRUE(Graph.end_recording());
- // State should not change on Queue2 now
- ASSERT_FALSE(Graph.end_recording(Queue2));
-
- // Testing vector begin and end
- ASSERT_TRUE(Graph.begin_recording({Queue, Queue2}));
- // Both shoudl now not have state changed
- ASSERT_FALSE(Graph.begin_recording(Queue));
- ASSERT_FALSE(Graph.begin_recording(Queue2));
-
- // End recording on both
- ASSERT_TRUE(Graph.end_recording({Queue, Queue2}));
- // Both shoudl now not have state changed
- ASSERT_FALSE(Graph.end_recording(Queue));
- ASSERT_FALSE(Graph.end_recording(Queue2));
-
- // First add one single queue
- ASSERT_TRUE(Graph.begin_recording(Queue));
- // Vector begin should still return true as Queue2 has state changed
- ASSERT_TRUE(Graph.begin_recording({Queue, Queue2}));
- // End recording on Queue2
- ASSERT_TRUE(Graph.end_recording(Queue2));
- // Vector end should still return true as Queue will have state changed
- ASSERT_TRUE(Graph.end_recording({Queue, Queue2}));
 }
 
 TEST_F(CommandGraphTest, GetCGCopy) {