Refactor raw arg update

Author: Ewan Crawford

sycl/source/detail/graph_impl.cpp

@@ -405,7 +405,7 @@ std::set<std::shared_ptr<node_impl>> graph_impl::getCGEdges(
     }
   }
 
-  return std::move(UniqueDeps);
+  return UniqueDeps;
 }
 
 void graph_impl::markCGMemObjs(
@@ -1563,7 +1563,7 @@ void exec_graph_impl::updateImpl(std::shared_ptr<node_impl> Node) {
     }
   }
 
-  UpdateDesc.hNewKernel = nullptr;
+  UpdateDesc.hNewKernel = UrKernel;
   UpdateDesc.numNewMemObjArgs = MemobjDescs.size();
   UpdateDesc.pNewMemObjArgList = MemobjDescs.data();
   UpdateDesc.numNewPointerArgs = PtrDescs.size();
@@ -1852,24 +1852,7 @@ void dynamic_parameter_impl::updateValue(const raw_kernel_arg *NewRawValue,
   size_t RawArgSize = NewRawValue->MArgSize;
   const void *RawArgData = NewRawValue->MArgData;
 
-  for (auto &[NodeWeak, ArgIndex] : MNodes) {
-    auto NodeShared = NodeWeak.lock();
-    if (NodeShared) {
-      dynamic_parameter_impl::updateCGArgValue(
-          NodeShared->MCommandGroup, ArgIndex, RawArgData, RawArgSize);
-    }
-  }
-
-  for (auto &DynCGInfo : MDynCGs) {
-    auto DynCG = DynCGInfo.DynCG.lock();
-    if (DynCG) {
-      auto &CG = DynCG->MKernels[DynCGInfo.CGIndex];
-      dynamic_parameter_impl::updateCGArgValue(CG, DynCGInfo.ArgIndex,
-                                               RawArgData, RawArgSize);
-    }
-  }
-
-  std::memcpy(MValueStorage.data(), RawArgData, RawArgSize);
+  updateValue(RawArgData, RawArgSize);
 }
 
 void dynamic_parameter_impl::updateValue(const void *NewValue, size_t Size) {
@@ -1987,7 +1970,6 @@ dynamic_command_group_impl::dynamic_command_group_impl(
 
 void dynamic_command_group_impl::finalizeCGFList(
     const std::vector<std::function<void(handler &)>> &CGFList) {
-  // True if kernels use sycl::nd_range, and false if using sycl::range
   for (size_t CGFIndex = 0; CGFIndex < CGFList.size(); CGFIndex++) {
     const auto &CGF = CGFList[CGFIndex];
     // Handler defined inside the loop so it doesn't appear to the runtime

sycl/source/detail/graph_impl.hpp

@@ -417,7 +417,7 @@ class node_impl : public std::enable_shared_from_this<node_impl> {
       throw sycl::exception(sycl::errc::invalid,
                             "Cannot update execution range of a node with an "
                             "execution range of different dimensions than what "
-                            "the node was original created with.");
+                            "the node was originally created with.");
     }
 
     NDRDesc = sycl::detail::NDRDescT{ExecutionRange};
@@ -438,7 +438,7 @@ class node_impl : public std::enable_shared_from_this<node_impl> {
       throw sycl::exception(sycl::errc::invalid,
                             "Cannot update execution range of a node with an "
                             "execution range of different dimensions than what "
-                            "the node was original created with.");
+                            "the node was originally created with.");
     }
 
     NDRDesc = sycl::detail::NDRDescT{ExecutionRange};
@@ -1173,11 +1173,6 @@ class graph_impl : public std::enable_shared_from_this<graph_impl> {
   /// @param Deps List of dependent nodes
   void addDepsToNode(std::shared_ptr<node_impl> Node,
                      std::vector<std::shared_ptr<node_impl>> &Deps) {
-    // Remove empty shared pointers from the list
-    auto EmptyElementIter =
-        std::remove(Deps.begin(), Deps.end(), std::shared_ptr<node_impl>());
-    Deps.erase(EmptyElementIter, Deps.end());
-
     if (!Deps.empty()) {
       for (auto &N : Deps) {
         N->registerSuccessor(Node);

sycl/source/handler.cpp

@@ -556,7 +556,10 @@ event handler::finalize() {
       // node can set it as a predecessor.
       auto DependentNode = GraphImpl->getLastInorderNode(MQueue);
       std::vector<std::shared_ptr<ext::oneapi::experimental::detail::node_impl>>
-          Deps = {DependentNode};
+          Deps;
+      if (DependentNode) {
+        Deps.push_back(DependentNode);
+      }
       NodeImpl = GraphImpl->add(NodeType, std::move(CommandGroup), Deps);
 
       // If we are recording an in-order queue remember the new node, so it
@@ -566,7 +569,11 @@ event handler::finalize() {
     } else {
       auto LastBarrierRecordedFromQueue = GraphImpl->getBarrierDep(MQueue);
       std::vector<std::shared_ptr<ext::oneapi::experimental::detail::node_impl>>
-          Deps = {LastBarrierRecordedFromQueue};
+          Deps;
+
+      if (LastBarrierRecordedFromQueue) {
+        Deps.push_back(LastBarrierRecordedFromQueue);
+      }
       NodeImpl = GraphImpl->add(NodeType, std::move(CommandGroup), Deps);
 
       if (NodeImpl->MCGType == sycl::detail::CGType::Barrier) {

sycl/test-e2e/Graph/Update/dyn_cgf_overwrite_range.cpp

@@ -5,9 +5,6 @@
 // Extra run to check for immediate-command-list in Level Zero
 // RUN: %if level_zero %{env SYCL_PI_LEVEL_ZERO_USE_IMMEDIATE_COMMANDLISTS=1 %{l0_leak_check} %{run} %t.out 2>&1 | FileCheck %s --implicit-check-not=LEAK %}
 
-// XFAIL: level_zero
-// XFAIL-TRACKER: OFNAAO-307
-
 // Tests how the nd-range of a node is overwritten by the active command-group
 
 #include "../graph_common.hpp"

sycl/test-e2e/Graph/Update/dyn_cgf_update_before_finalize.cpp

@@ -5,9 +5,6 @@
 // Extra run to check for immediate-command-list in Level Zero
 // RUN: %if level_zero %{env SYCL_PI_LEVEL_ZERO_USE_IMMEDIATE_COMMANDLISTS=1 %{l0_leak_check} %{run} %t.out 2>&1 | FileCheck %s --implicit-check-not=LEAK %}
 
-// XFAIL: level_zero
-// XFAIL-TRACKER: OFNAAO-307
-
 // Tests updating a dynamic command-group node after it has been added to
 // a graph but before the graph has been finalized