From 0318696128e6962b1fb05e950eb904955cab49bb Mon Sep 17 00:00:00 2001 From: Maxime France-Pillois Date: Wed, 6 Mar 2024 14:26:30 +0000 Subject: [PATCH] Pass prop-list to `executable_command_graph` constructor + typos --- sycl/include/sycl/ext/oneapi/experimental/graph.hpp | 5 +++-- sycl/source/detail/graph_impl.cpp | 13 ++++--------- sycl/source/detail/graph_impl.hpp | 12 +++++++----- sycl/test-e2e/Graph/event_profiling_info.cpp | 4 ++-- 4 files changed, 16 insertions(+), 18 deletions(-) diff --git a/sycl/include/sycl/ext/oneapi/experimental/graph.hpp b/sycl/include/sycl/ext/oneapi/experimental/graph.hpp index 029f68a032ba4..1ffbf3c6d83c3 100644 --- a/sycl/include/sycl/ext/oneapi/experimental/graph.hpp +++ b/sycl/include/sycl/ext/oneapi/experimental/graph.hpp @@ -349,10 +349,11 @@ class __SYCL_EXPORT executable_command_graph { /// Constructor used by internal runtime. /// @param Graph Detail implementation class to construct with. /// @param Ctx Context to use for graph. - /// @param EnableProfiling Enable graph profiling. + /// @param PropList Optional list of properties to pass. executable_command_graph(const std::shared_ptr &Graph, const sycl::context &Ctx, - const bool EnableProfiling = false); + const property_list &PropList = {}); + template friend decltype(Obj::impl) diff --git a/sycl/source/detail/graph_impl.cpp b/sycl/source/detail/graph_impl.cpp index 83205fb36382e..a7a9d0c40a5f7 100644 --- a/sycl/source/detail/graph_impl.cpp +++ b/sycl/source/detail/graph_impl.cpp @@ -683,7 +683,7 @@ void exec_graph_impl::createCommandBuffers( sycl::detail::pi::PiExtCommandBuffer OutCommandBuffer; sycl::detail::pi::PiExtCommandBufferDesc Desc{ pi_ext_structure_type::PI_EXT_STRUCTURE_TYPE_COMMAND_BUFFER_DESC, nullptr, - pi_bool(Partition->MIsInOrderGraph & !MEnableProfiling), + pi_bool(Partition->MIsInOrderGraph && !MEnableProfiling), pi_bool(MEnableProfiling)}; auto ContextImpl = sycl::detail::getSyclObjImpl(MContext); const sycl::detail::PluginPtr &Plugin = ContextImpl->getPlugin(); @@ -1163,12 +1163,8 @@ modifiable_command_graph::finalize(const sycl::property_list &PropList) const { // Graph is read and written in this scope so we lock // this graph with full priviledges. graph_impl::WriteLock Lock(impl->MMutex); - bool EnableProfiling = false; - if (PropList.has_property()) { - EnableProfiling = true; - } return command_graph{ - this->impl, this->impl->getContext(), EnableProfiling}; + this->impl, this->impl->getContext(), PropList}; } bool modifiable_command_graph::begin_recording(queue &RecordingQueue) { @@ -1283,9 +1279,8 @@ std::vector modifiable_command_graph::get_root_nodes() const { executable_command_graph::executable_command_graph( const std::shared_ptr &Graph, const sycl::context &Ctx, - const bool EnableProfiling) - : impl(std::make_shared(Ctx, Graph, - EnableProfiling)) { + const property_list &PropList) + : impl(std::make_shared(Ctx, Graph, PropList)) { finalizeImpl(); // Create backend representation for executable graph } diff --git a/sycl/source/detail/graph_impl.hpp b/sycl/source/detail/graph_impl.hpp index d47113a9010c4..64e86a9bf44fc 100644 --- a/sycl/source/detail/graph_impl.hpp +++ b/sycl/source/detail/graph_impl.hpp @@ -568,8 +568,8 @@ class partition { MPiCommandBuffers; /// List of predecessors to this partition. std::vector> MPredecessors; - /// True is the graph of this partition is a single path graph - /// and InOrder optmization can be applied on it. + /// True if the graph of this partition is a single path graph + /// and in-order optmization can be applied on it. bool MIsInOrderGraph = false; /// @return True if the partition contains a host task @@ -1061,12 +1061,14 @@ class exec_graph_impl { /// nodes). /// @param Context Context to create graph with. /// @param GraphImpl Modifiable graph implementation to create with. - /// @param EnableProfiling Enable graph profiling. + /// @param PropList List of properties for constructing this object. exec_graph_impl(sycl::context Context, const std::shared_ptr &GraphImpl, - const bool EnableProfiling = false) + const property_list &PropList) : MSchedule(), MGraphImpl(GraphImpl), MPiSyncPoints(), MContext(Context), - MRequirements(), MExecutionEvents(), MEnableProfiling(EnableProfiling) { + MRequirements(), MExecutionEvents(), + MEnableProfiling( + PropList.has_property()) { // Copy nodes from GraphImpl and merge any subgraph nodes into this graph. duplicateNodes(); } diff --git a/sycl/test-e2e/Graph/event_profiling_info.cpp b/sycl/test-e2e/Graph/event_profiling_info.cpp index 28ad32bf4ed79..24fc478223caa 100644 --- a/sycl/test-e2e/Graph/event_profiling_info.cpp +++ b/sycl/test-e2e/Graph/event_profiling_info.cpp @@ -130,8 +130,8 @@ int main() { KernelGraph.end_recording(Queue); - // The profiling is not available with the in-order optimization. - // We therefore disable this optimization. + // The `enable_profiling` property must be passed to the finalize function + // in order to query profiling information. auto CopyGraphExec = CopyGraph.finalize(exp_ext::property::graph::enable_profiling{}); auto KernelGraphExec =