[UR][Graph] Strengthen in-order command-buffer property (#18444)

Closes issue intel/llvm#18330 to enhance the
semantics of the UR command-buffer in-order property such that
sync-point dependencies will always be ignored.

Also fixes up an artifact in the UR command-buffer spec where it says
that descriptors to the command-buffer creation API can be optional,
when after intel/llvm#17058 they are mandatory.

Author: Ewan Crawford

unified-runtime/include/ur_api.h

@@ -53,14 +53,14 @@ Command-Buffer Creation
 --------------------------------------------------------------------------------
 
 Command-Buffers are tied to a specific ${x}_context_handle_t and
-${x}_device_handle_t. ${x}CommandBufferCreateExp optionally takes a descriptor
+${x}_device_handle_t. ${x}CommandBufferCreateExp takes a descriptor
 to provide additional properties for how the command-buffer should be
 constructed. The members defined in ${x}_exp_command_buffer_desc_t are:
 
 * ``isUpdatable``, which should be set to ``true`` to support :ref:`updating
   command-buffer commands`.
-* ``isInOrder``, which should be set to ``true`` to enable commands enqueued to
-  a command-buffer to be executed in an in-order fashion where possible.
+* ``isInOrder``, which should be set to ``true`` to enforce commands appended
+  to a command-buffer to be executed in an in-order fashion.
 * ``enableProfiling``, which should be set to ``true`` to enable profiling of
   the command-buffer.
 
@@ -108,8 +108,9 @@ Sync-Points
 A sync-point is a value which represents a command inside of a command-buffer
 which is returned from command-buffer append function calls. These can be
 optionally passed to these functions to define execution dependencies on other
-commands within the command-buffer. Sync-points passed to functions may be
-ignored if the command-buffer was created in-order.
+commands within the command-buffer. Both wait-list and return sync-point
+parameters to append functions are ignored if the command-buffer was created
+with the in-order property.
 
 Sync-points are unique and valid for use only within the command-buffer they
 were obtained from.
@@ -550,6 +551,9 @@ Changelog
 +-----------+-------------------------------------------------------+
 | 1.11      | Support native commands.                              |
 +-----------+-------------------------------------------------------+
+| 1.12      | Strengthen in-order property such that sync-points    |
+|           | parameters to append APIs are ignored.                |
++-----------+-------------------------------------------------------+
 
 Contributors
 --------------------------------------------------------------------------------

unified-runtime/scripts/core/EXP-COMMAND-BUFFER.rst

@@ -143,7 +143,7 @@ members:
       desc: "[in] Commands in a finalized command-buffer can be updated."
     - type: $x_bool_t
       name: isInOrder
-      desc: "[in] Commands in a command-buffer may be executed in-order without explicit dependencies."
+      desc: "[in] Commands in a command-buffer will execute in-order, explicit sync-point dependencies are ignored."
     - type: $x_bool_t
       name: enableProfiling
       desc: "[in] Command-buffer profiling is enabled."
@@ -375,7 +375,7 @@ params:
     - type: "const $x_exp_command_buffer_sync_point_t*"
       name: pSyncPointWaitList
       desc: "[in][optional] A list of sync points that this command depends on.
-      May be ignored if command-buffer is in-order."
+      Will be ignored if command-buffer is in-order."
     - type: uint32_t
       name: numEventsInWaitList
       desc: "[in] Size of the event wait list."
@@ -440,7 +440,7 @@ params:
     - type: "const $x_exp_command_buffer_sync_point_t*"
       name: pSyncPointWaitList
       desc: "[in][optional] A list of sync points that this command depends on.
-      May be ignored if command-buffer is in-order."
+      Will be ignored if command-buffer is in-order."
     - type: uint32_t
       name: numEventsInWaitList
       desc: "[in] Size of the event wait list."
@@ -502,7 +502,7 @@ params:
     - type: "const $x_exp_command_buffer_sync_point_t*"
       name: pSyncPointWaitList
       desc: "[in][optional] A list of sync points that this command depends on.
-      May be ignored if command-buffer is in-order."
+      Will be ignored if command-buffer is in-order."
     - type: uint32_t
       name: numEventsInWaitList
       desc: "[in] Size of the event wait list."
@@ -569,7 +569,7 @@ params:
     - type: "const $x_exp_command_buffer_sync_point_t*"
       name: pSyncPointWaitList
       desc: "[in][optional] A list of sync points that this command depends on.
-      May be ignored if command-buffer is in-order."
+      Will be ignored if command-buffer is in-order."
     - type: uint32_t
       name: numEventsInWaitList
       desc: "[in] Size of the event wait list."
@@ -628,7 +628,7 @@ params:
     - type: "const $x_exp_command_buffer_sync_point_t*"
       name: pSyncPointWaitList
       desc: "[in][optional] A list of sync points that this command depends on.
-      May be ignored if command-buffer is in-order."
+      Will be ignored if command-buffer is in-order."
     - type: uint32_t
       name: numEventsInWaitList
       desc: "[in] Size of the event wait list."
@@ -687,7 +687,7 @@ params:
     - type: "const $x_exp_command_buffer_sync_point_t*"
       name: pSyncPointWaitList
       desc: "[in][optional] A list of sync points that this command depends on.
-      May be ignored if command-buffer is in-order."
+      Will be ignored if command-buffer is in-order."
     - type: uint32_t
       name: numEventsInWaitList
       desc: "[in] Size of the event wait list."
@@ -761,7 +761,7 @@ params:
     - type: "const $x_exp_command_buffer_sync_point_t*"
       name: pSyncPointWaitList
       desc: "[in][optional] A list of sync points that this command depends on.
-      May be ignored if command-buffer is in-order."
+      Will be ignored if command-buffer is in-order."
     - type: uint32_t
       name: numEventsInWaitList
       desc: "[in] Size of the event wait list."
@@ -835,7 +835,7 @@ params:
     - type: "const $x_exp_command_buffer_sync_point_t*"
       name: pSyncPointWaitList
       desc: "[in][optional] A list of sync points that this command depends on.
-      May be ignored if command-buffer is in-order."
+      Will be ignored if command-buffer is in-order."
     - type: uint32_t
       name: numEventsInWaitList
       desc: "[in] Size of the event wait list."
@@ -909,7 +909,7 @@ params:
     - type: "const $x_exp_command_buffer_sync_point_t*"
       name: pSyncPointWaitList
       desc: "[in][optional] A list of sync points that this command depends on.
-      May be ignored if command-buffer is in-order."
+      Will be ignored if command-buffer is in-order."
     - type: uint32_t
       name: numEventsInWaitList
       desc: "[in] Size of the event wait list."
@@ -971,7 +971,7 @@ params:
     - type: "const $x_exp_command_buffer_sync_point_t*"
       name: pSyncPointWaitList
       desc: "[in][optional] A list of sync points that this command depends on.
-      May be ignored if command-buffer is in-order."
+      Will be ignored if command-buffer is in-order."
     - type: uint32_t
       name: numEventsInWaitList
       desc: "[in] Size of the event wait list."
@@ -1032,7 +1032,7 @@ params:
     - type: "const $x_exp_command_buffer_sync_point_t*"
       name: pSyncPointWaitList
       desc: "[in][optional] A list of sync points that this command depends on.
-      May be ignored if command-buffer is in-order."
+      Will be ignored if command-buffer is in-order."
     - type: uint32_t
       name: numEventsInWaitList
       desc: "[in] Size of the event wait list."
@@ -1095,7 +1095,7 @@ params:
     - type: "const $x_exp_command_buffer_sync_point_t*"
       name: pSyncPointWaitList
       desc: "[in][optional] A list of sync points that this command depends on.
-      May be ignored if command-buffer is in-order."
+      Will be ignored if command-buffer is in-order."
     - type: uint32_t
       name: numEventsInWaitList
       desc: "[in] Size of the event wait list."
@@ -1166,7 +1166,7 @@ params:
     - type: "const $x_exp_command_buffer_sync_point_t*"
       name: pSyncPointWaitList
       desc: "[in][optional] A list of sync points that this command depends on.
-      May be ignored if command-buffer is in-order."
+      Will be ignored if command-buffer is in-order."
     - type: "$x_exp_command_buffer_sync_point_t*"
       name: pSyncPoint
       desc: "[out][optional] Sync point associated with this command."

unified-runtime/scripts/core/exp-command-buffer.yml

@@ -56,10 +56,11 @@ ur_result_t commandHandleDestroy(
 } // end anonymous namespace
 
 ur_exp_command_buffer_handle_t_::ur_exp_command_buffer_handle_t_(
-    ur_context_handle_t Context, ur_device_handle_t Device, bool IsUpdatable)
+    ur_context_handle_t Context, ur_device_handle_t Device, bool IsUpdatable,
+    bool IsInOrder)
     : handle_base(), Context(Context), Device(Device), IsUpdatable(IsUpdatable),
-      CudaGraph{nullptr}, CudaGraphExec{nullptr}, RefCount{1},
-      NextSyncPoint{0} {
+      IsInOrder(IsInOrder), CudaGraph{nullptr}, CudaGraphExec{nullptr},
+      RefCount{1}, NextSyncPoint{0} {
   urContextRetain(Context);
 }
 
@@ -151,11 +152,24 @@ static ur_result_t getNodesFromSyncPoints(
   // the event associated with each sync-point
   auto SyncPoints = CommandBuffer->SyncPoints;
 
+  // If command-buffer is in-order use last node in ordered map, and return
+  // early as other user passed sync-points will be redundant for scheduling.
+  if (CommandBuffer->IsInOrder && !SyncPoints.empty()) {
+    auto LastNode = std::prev(SyncPoints.end());
+    CuNodesList.push_back(LastNode->second);
+    return UR_RESULT_SUCCESS;
+  }
+
   // For each sync-point add associated CUDA graph node to the return list.
   for (size_t i = 0; i < NumSyncPointsInWaitList; i++) {
     if (auto NodeHandle = SyncPoints.find(SyncPointWaitList[i]);
         NodeHandle != SyncPoints.end()) {
-      CuNodesList.push_back(NodeHandle->second);
+      auto DepNode = NodeHandle->second;
+      // Cuda driver API won't let you add duplicates to the dependency list
+      if (std::find(CuNodesList.begin(), CuNodesList.end(), DepNode) ==
+          CuNodesList.end()) {
+        CuNodesList.push_back(DepNode);
+      }
     } else {
       return UR_RESULT_ERROR_INVALID_VALUE;
     }
@@ -346,9 +360,10 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferCreateExp(
     const ur_exp_command_buffer_desc_t *pCommandBufferDesc,
     ur_exp_command_buffer_handle_t *phCommandBuffer) {
   const bool IsUpdatable = pCommandBufferDesc->isUpdatable;
+  const bool IsInOrder = pCommandBufferDesc->isInOrder;
   try {
-    *phCommandBuffer =
-        new ur_exp_command_buffer_handle_t_(hContext, hDevice, IsUpdatable);
+    *phCommandBuffer = new ur_exp_command_buffer_handle_t_(
+        hContext, hDevice, IsUpdatable, IsInOrder);
   } catch (const std::bad_alloc &) {
     return UR_RESULT_ERROR_OUT_OF_HOST_MEMORY;
   } catch (...) {