diff --git a/sycl/doc/extensions/experimental/sycl_ext_oneapi_graph.asciidoc b/sycl/doc/extensions/experimental/sycl_ext_oneapi_graph.asciidoc index a9690ab73764c..2b0f8ae3e9f45 100644 --- a/sycl/doc/extensions/experimental/sycl_ext_oneapi_graph.asciidoc +++ b/sycl/doc/extensions/experimental/sycl_ext_oneapi_graph.asciidoc @@ -618,12 +618,12 @@ public: command_graph finalize(const property_list& propList = {}) const; - bool begin_recording(queue& recordingQueue, const property_list& propList = {}); - bool begin_recording(const std::vector& recordingQueues, const property_list& propList = {}); + void begin_recording(queue& recordingQueue, const property_list& propList = {}); + void begin_recording(const std::vector& recordingQueues, const property_list& propList = {}); - bool end_recording(); - bool end_recording(queue& recordingQueue); - bool end_recording(const std::vector& recordingQueues); + void end_recording(); + void end_recording(queue& recordingQueue); + void end_recording(const std::vector& 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 <>. -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& 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 <>. -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& recordingQueues) +void end_recording(const std::vector& 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 diff --git a/sycl/include/sycl/ext/oneapi/experimental/graph.hpp b/sycl/include/sycl/ext/oneapi/experimental/graph.hpp index 5cf6b7d9ee761..4e9f4c103f945 100644 --- a/sycl/include/sycl/ext/oneapi/experimental/graph.hpp +++ b/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 &RecordingQueues); + /// @param PropList Property list used to pass properties for recording. + void begin_recording(const std::vector &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 &RecordingQueues); + void end_recording(const std::vector &RecordingQueues); /// Synchronous operation that writes a DOT formatted description of the graph /// to the provided path. By default, this includes the graph topology, node diff --git a/sycl/source/detail/graph_impl.cpp b/sycl/source/detail/graph_impl.cpp index 5d266bf55a4a2..4cf1c683156c1 100644 --- a/sycl/source/detail/graph_impl.cpp +++ b/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 &RecordingQueues) { - bool QueueStateChanged = false; +void modifiable_command_graph::begin_recording( + const std::vector &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 &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, diff --git a/sycl/test-e2e/Graph/RecordReplay/return_values.cpp b/sycl/test-e2e/Graph/RecordReplay/return_values.cpp deleted file mode 100644 index 1e3e7e6bd154f..0000000000000 --- a/sycl/test-e2e/Graph/RecordReplay/return_values.cpp +++ /dev/null @@ -1,34 +0,0 @@ -// RUN: %{build} -o %t.out -// RUN: %{run} %t.out -// Extra run to check for leaks in Level Zero using UR_L0_LEAKS_DEBUG -// RUN: %if level_zero %{env SYCL_PI_LEVEL_ZERO_USE_IMMEDIATE_COMMANDLISTS=0 %{l0_leak_check} %{run} %t.out 2>&1 | FileCheck %s --implicit-check-not=LEAK %} -// Extra run to check for immediate-command-list in Level Zero -// RUN: %if level_zero && linux %{env SYCL_PI_LEVEL_ZERO_USE_IMMEDIATE_COMMANDLISTS=1 %{l0_leak_check} %{run} %t.out 2>&1 | FileCheck %s --implicit-check-not=LEAK %} - -// Tests the return values from queue graph functions which change the -// internal queue state. - -#include "../graph_common.hpp" - -int main() { - queue Queue; - - exp_ext::command_graph Graph{Queue.get_context(), Queue.get_device()}; - - bool ChangedState = Graph.end_recording(); - assert(ChangedState == false); - - ChangedState = Graph.begin_recording(Queue); - assert(ChangedState == true); - - ChangedState = Graph.begin_recording(Queue); - assert(ChangedState == false); - - ChangedState = Graph.end_recording(); - assert(ChangedState == true); - - ChangedState = Graph.end_recording(); - assert(ChangedState == false); - - return 0; -} diff --git a/sycl/test/abi/sycl_symbols_linux.dump b/sycl/test/abi/sycl_symbols_linux.dump index 28944dad6adba..96cd0e1d25fe6 100644 --- a/sycl/test/abi/sycl_symbols_linux.dump +++ b/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 diff --git a/sycl/unittests/Extensions/CommandGraph/CommandGraph.cpp b/sycl/unittests/Extensions/CommandGraph/CommandGraph.cpp index 63b5b2a04de05..20b95f99a2d14 100644 --- a/sycl/unittests/Extensions/CommandGraph/CommandGraph.cpp +++ b/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) {