Merge pull request #1790 from arcaneframework/dev/gg-disallow-reusing-command

Disallow reusing the command to launch two kernels

Author: grospelliergilles

arcane/ceapart/src/arcane/tests/CartesianMeshTestUtils.cc

@@ -1,11 +1,11 @@
 // -*- tab-width: 2; indent-tabs-mode: nil; coding: utf-8-with-signature -*-
 //-----------------------------------------------------------------------------
-// Copyright 2000-2023 CEA (www.cea.fr) IFPEN (www.ifpenergiesnouvelles.com)
+// Copyright 2000-2024 CEA (www.cea.fr) IFPEN (www.ifpenergiesnouvelles.com)
 // See the top-level COPYRIGHT file for details.
 // SPDX-License-Identifier: Apache-2.0
 //-----------------------------------------------------------------------------
 /*---------------------------------------------------------------------------*/
-/* CartesianMeshTestUtils.cc                                   (C) 2000-2023 */
+/* CartesianMeshTestUtils.cc                                   (C) 2000-2024 */
 /*                                                                           */
 /* Fonctions utilitaires pour les tests de 'CartesianMesh'.                  */
 /*---------------------------------------------------------------------------*/
@@ -245,19 +245,19 @@ _testDirCellAccelerator()
   CellDirectionMng cdm2;
   CellDirectionMng cdm3;
 
-  auto queue = m_accelerator_mng->defaultQueue();
-  auto command = makeCommand(*queue);
+  auto queue = m_accelerator_mng->queue();
 
   VariableCellInt32 dummy_var(VariableBuildInfo(mesh, "DummyCellVariable"));
   dummy_var.fill(0);
-  auto inout_dummy_var = viewInOut(command, dummy_var);
 
   for (Integer idir = 0; idir < nb_dir; ++idir) {
     CellDirectionMng cdm(m_cartesian_mesh->cellDirection(idir));
     cdm2 = m_cartesian_mesh->cellDirection(idir);
     cdm3 = cdm;
     info() << "ACCELERATOR_DIRECTION=" << idir << " Cells=" << cdm.allCells().name();
     _checkItemGroupIsSorted(cdm.allCells());
+    auto command = makeCommand(queue);
+    auto inout_dummy_var = viewInOut(command, dummy_var);
     command << RUNCOMMAND_ENUMERATE(Cell, icell, cdm.allCells())
     {
       DirCellLocalId dir_cell(cdm.dirCellId(icell));

arcane/src/arcane/accelerator/core/RunCommandImpl.cc

@@ -17,6 +17,7 @@
 #include "arcane/utils/ForLoopTraceInfo.h"
 #include "arcane/utils/ConcurrencyUtils.h"
 #include "arcane/utils/PlatformUtils.h"
+#include "arcane/utils/ValueConvert.h"
 
 #include "arcane/accelerator/core/Runner.h"
 #include "arcane/accelerator/core/internal/IRunQueueEventImpl.h"
@@ -99,6 +100,9 @@ _init()
 
   m_start_event = _createEvent();
   m_stop_event = _createEvent();
+
+  if (auto v = Convert::Type<Int32>::tryParseFromEnvironment("ARCANE_ACCELERATOR_ALLOW_REUSE_COMMAND", true))
+    m_is_allow_reuse_command = (v.value() != 0);
 }
 
 /*---------------------------------------------------------------------------*/
@@ -107,7 +111,9 @@ _init()
 RunCommandImpl* RunCommandImpl::
 create(RunQueueImpl* r)
 {
-  return r->_internalCreateOrGetRunCommandImpl();
+  RunCommandImpl* c = r->_internalCreateOrGetRunCommandImpl();
+  c->_reset();
+  return c;
 }
 
 /*---------------------------------------------------------------------------*/
@@ -118,6 +124,12 @@ create(RunQueueImpl* r)
 void RunCommandImpl::
 notifyBeginLaunchKernel()
 {
+  if (m_has_been_launched) {
+    if (!m_is_allow_reuse_command)
+      ARCANE_FATAL("Command has already been launched. You can not re-use the same command.\n"
+                   "  You can temporarily allow it if you set environment variable\n"
+                   "    ARCANE_ACCELERATOR_ALLOW_REUSE_COMMAND to 1\n");
+  }
   IRunQueueStream* stream = internalStream();
   stream->notifyBeginLaunchKernel(*this);
   // TODO: utiliser la bonne stream en séquentiel
@@ -193,8 +205,6 @@ notifyEndExecuteKernel()
     ForLoopTraceInfo flti(traceInfo(), kernelName());
     ProfilingRegistry::_threadLocalForLoopInstance()->merge(*exec_info, flti);
   }
-
-  _reset();
 }
 
 /*---------------------------------------------------------------------------*/

arcane/src/arcane/accelerator/core/RunQueueImpl.cc

@@ -191,6 +191,7 @@ _internalCreateOrGetRunCommandImpl()
   else {
     p = RunCommand::_internalCreateImpl(this);
   }
+  p->_reset();
   m_active_run_command_list.add(p);
   return p;
 }

arcane/src/arcane/accelerator/core/internal/RunCommandImpl.h

@@ -38,6 +38,7 @@ namespace Arcane::Accelerator::impl
 class RunCommandImpl
 {
   friend RunCommand;
+  friend RunQueueImpl;
 
  public:
 
@@ -110,6 +111,15 @@ class RunCommandImpl
   //! Indique si la commande s'exécute sur accélérateur
   const bool m_use_accelerator = false;
 
+  /*!
+  * \brief Indique si on autorise à utiliser plusieurs fois la même commande.
+   *
+   * Normalement cela est interdit mais avant novembre 2024, il n'y avait pas
+   * de mécanisme pour détecter cela. On peut donc temporairement autoriser
+   * cela et dans un on supprimera cette possibilité.
+   */
+  bool m_is_allow_reuse_command = false;
+
  private:
 
   void _freePools();

arcane/src/arcane/tests/accelerator/AcceleratorViewsUnitTest.cc

@@ -55,7 +55,8 @@ class AcceleratorViewsUnitTest
 
  private:
 
-  ax::Runner* m_runner = nullptr;
+  ax::Runner m_runner;
+  ax::RunQueue m_queue;
   VariableCellArrayReal m_cell_array1;
   VariableCellArrayReal m_cell_array2;
   VariableCellReal2 m_cell1_real2;
@@ -136,7 +137,8 @@ AcceleratorViewsUnitTest::
 void AcceleratorViewsUnitTest::
 initializeTest()
 {
-  m_runner = subDomain()->acceleratorMng()->defaultRunner();
+  m_runner = subDomain()->acceleratorMng()->runner();
+  m_queue = subDomain()->acceleratorMng()->queue();
 
   m_cell_array1.resize(12);
   m_cell_array2.resize(12);
@@ -217,64 +219,70 @@ _executeTest1()
 {
   info() << "Test1";
   auto queue = makeQueue(m_runner);
-  auto command = makeCommand(queue);
 
   Integer dim2_size = m_cell_array1.arraySize();
 
   {
     int seed = 37;
     _setCellArrayValue(seed);
 
-    auto in_cell_array1 = ax::viewIn(command, m_cell_array1);
-    auto out_cell_array2 = ax::viewOut(command, m_cell_array2);
-    auto in_partial_cell_array1 = ax::viewIn(command, m_partial_cell_array1);
-    auto out_partial_cell_array2 = ax::viewOut(command, m_partial_cell_array2);
-    auto inout_partial_cell_array2 = ax::viewInOut(command, m_partial_cell_array2);
-
-    auto in_partial_cell1 = ax::viewIn(command, m_partial_cell1);
-    auto out_partial_cell2 = ax::viewOut(command, m_partial_cell2);
-    auto inout_partial_cell2 = ax::viewOut(command, m_partial_cell2);
-
-    auto in_partial_cell1_real3 = ax::viewIn(command, m_partial_cell1_real3);
-    auto out_partial_cell2_real3 = ax::viewOut(command, m_partial_cell2_real3);
-    auto inout_partial_cell2_real3 = ax::viewInOut(command, m_partial_cell2_real3);
-
-    command << RUNCOMMAND_ENUMERATE (CellLocalId, vi, allCells())
     {
-      out_cell_array2[vi].copy(in_cell_array1[vi]);
-    };
+      auto command = makeCommand(m_queue);
+      auto in_cell_array1 = viewIn(command, m_cell_array1);
+      auto out_cell_array2 = viewOut(command, m_cell_array2);
+
+      command << RUNCOMMAND_ENUMERATE (CellLocalId, vi, allCells())
+      {
+        out_cell_array2[vi].copy(in_cell_array1[vi]);
+      };
+    }
 
     _checkCellArrayValue("View1");
 
-    command << RUNCOMMAND_ENUMERATE (IteratorWithIndex<CellLocalId>, vi, m_partial_cell_array1.itemGroup())
     {
-      CellEnumeratorIndex iter_index(vi.index());
-      CellLocalId cell_lid(vi.value());
-      out_partial_cell_array2[iter_index].copy(in_cell_array1[cell_lid]);
-      out_partial_cell_array2[iter_index][0] = in_partial_cell_array1[cell_lid][1];
-      Real3 xyz(in_partial_cell1_real3[iter_index].y, in_partial_cell1_real3[iter_index].z, in_partial_cell1_real3[iter_index].x);
-      Int32 modulo = vi.index() % 4;
-      if (modulo == 3) {
-        out_partial_cell_array2[iter_index][1] = in_partial_cell1[iter_index];
-        out_partial_cell2[iter_index] = in_partial_cell1[iter_index];
-        out_partial_cell2_real3[iter_index] = xyz;
-      }
-      else if (modulo == 2) {
-        inout_partial_cell_array2[cell_lid][1] = in_partial_cell1[cell_lid];
-        inout_partial_cell2[cell_lid] = in_partial_cell1[iter_index];
-        inout_partial_cell2_real3[cell_lid] = xyz;
-      }
-      else if (modulo == 1) {
-        out_partial_cell_array2[cell_lid][1] = in_partial_cell1[iter_index];
-        out_partial_cell2[cell_lid] = in_partial_cell1[iter_index];
-        out_partial_cell2_real3[cell_lid] = xyz;
-      }
-      else {
-        inout_partial_cell_array2[iter_index][1] = in_partial_cell1[cell_lid];
-        inout_partial_cell2[iter_index] = in_partial_cell1[iter_index];
-        inout_partial_cell2_real3[iter_index] = xyz;
-      }
-    };
+      auto command = makeCommand(m_queue);
+      auto in_cell_array1 = viewIn(command, m_cell_array1);
+      auto in_partial_cell_array1 = viewIn(command, m_partial_cell_array1);
+      auto out_partial_cell_array2 = viewOut(command, m_partial_cell_array2);
+      auto inout_partial_cell_array2 = viewInOut(command, m_partial_cell_array2);
+
+      auto in_partial_cell1 = viewIn(command, m_partial_cell1);
+      auto out_partial_cell2 = viewOut(command, m_partial_cell2);
+      auto inout_partial_cell2 = viewOut(command, m_partial_cell2);
+
+      auto in_partial_cell1_real3 = viewIn(command, m_partial_cell1_real3);
+      auto out_partial_cell2_real3 = viewOut(command, m_partial_cell2_real3);
+      auto inout_partial_cell2_real3 = viewInOut(command, m_partial_cell2_real3);
+      command << RUNCOMMAND_ENUMERATE (IteratorWithIndex<CellLocalId>, vi, m_partial_cell_array1.itemGroup())
+      {
+        CellEnumeratorIndex iter_index(vi.index());
+        CellLocalId cell_lid(vi.value());
+        out_partial_cell_array2[iter_index].copy(in_cell_array1[cell_lid]);
+        out_partial_cell_array2[iter_index][0] = in_partial_cell_array1[cell_lid][1];
+        Real3 xyz(in_partial_cell1_real3[iter_index].y, in_partial_cell1_real3[iter_index].z, in_partial_cell1_real3[iter_index].x);
+        Int32 modulo = vi.index() % 4;
+        if (modulo == 3) {
+          out_partial_cell_array2[iter_index][1] = in_partial_cell1[iter_index];
+          out_partial_cell2[iter_index] = in_partial_cell1[iter_index];
+          out_partial_cell2_real3[iter_index] = xyz;
+        }
+        else if (modulo == 2) {
+          inout_partial_cell_array2[cell_lid][1] = in_partial_cell1[cell_lid];
+          inout_partial_cell2[cell_lid] = in_partial_cell1[iter_index];
+          inout_partial_cell2_real3[cell_lid] = xyz;
+        }
+        else if (modulo == 1) {
+          out_partial_cell_array2[cell_lid][1] = in_partial_cell1[iter_index];
+          out_partial_cell2[cell_lid] = in_partial_cell1[iter_index];
+          out_partial_cell2_real3[cell_lid] = xyz;
+        }
+        else {
+          inout_partial_cell_array2[iter_index][1] = in_partial_cell1[cell_lid];
+          inout_partial_cell2[iter_index] = in_partial_cell1[iter_index];
+          inout_partial_cell2_real3[iter_index] = xyz;
+        }
+      };
+    }
     info() << "Check Partial values";
     ENUMERATE_ (Cell, iter, m_partial_cell_array1.itemGroup()) {
       CellEnumeratorIndex iter_index(iter.index());
@@ -290,6 +298,8 @@ _executeTest1()
     int seed = 23;
     _setCellArrayValue(seed);
 
+    auto command = makeCommand(m_queue);
+
     auto in_cell_array1 = viewIn(command, m_cell_array1);
     auto out_cell_array2 = viewOut(command, m_cell_array2);
 
@@ -305,6 +315,8 @@ _executeTest1()
     int seed = 53;
     _setCellArrayValue(seed);
 
+    auto command = makeCommand(m_queue);
+
     auto in_cell_array1 = viewInOut(command, m_cell_array1);
     auto out_cell_array2 = viewOut(command, m_cell_array2);
 
@@ -320,6 +332,8 @@ _executeTest1()
     int seed = 93;
     _setCellArrayValue(seed);
 
+    auto command = makeCommand(m_queue);
+
     auto in_cell_array1 = ax::viewIn(command, m_cell_array1);
     auto out_cell_array2 = ax::viewInOut(command, m_cell_array2);
 
@@ -335,6 +349,8 @@ _executeTest1()
     int seed = 43;
     _setCellArrayValue(seed);
 
+    auto command = makeCommand(m_queue);
+
     auto inout_cell_array1 = ax::viewInOut(command, m_cell_array1);
     auto out_cell_array2 = ax::viewInOut(command, m_cell_array2);
 
@@ -686,7 +702,7 @@ _executeTestMemoryCopy()
   info() << "Execute Test MemoryCopy";
   eMemoryRessource source_mem = eMemoryRessource::Host;
   eMemoryRessource dest_mem = eMemoryRessource::Host;
-  if (ax::impl::isAcceleratorPolicy(m_runner->executionPolicy()))
+  if (ax::impl::isAcceleratorPolicy(m_runner.executionPolicy()))
     dest_mem = eMemoryRessource::Device;
 
   const int nb_value = 100000;