Merge pull request #2462 from Bensuo/cmd-buf_update_errors

Improve specification of command-buffer update errors

Author: kbenzie

include/ur_api.h

@@ -8508,8 +8508,8 @@ typedef struct ur_exp_command_buffer_command_handle_t_ *ur_exp_command_buffer_co
 ///         + `NULL == phCommandBuffer`
 ///     - ::UR_RESULT_ERROR_INVALID_CONTEXT
 ///     - ::UR_RESULT_ERROR_INVALID_DEVICE
-///     - ::UR_RESULT_ERROR_INVALID_OPERATION
-///         + If `pCommandBufferDesc->isUpdatable` is true and `hDevice` does not support UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_SUPPORT_EXP.
+///     - ::UR_RESULT_ERROR_UNSUPPORTED_FEATURE
+///         + If `pCommandBufferDesc->isUpdatable` is true and `hDevice` returns 0 for the ::UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_CAPABILITIES_EXP query.
 ///     - ::UR_RESULT_ERROR_OUT_OF_HOST_MEMORY
 ///     - ::UR_RESULT_ERROR_OUT_OF_RESOURCES
 UR_APIEXPORT ur_result_t UR_APICALL
@@ -9315,11 +9315,17 @@ urCommandBufferReleaseCommandExp(
 ///     - ::UR_RESULT_ERROR_INVALID_NULL_POINTER
 ///         + `NULL == pUpdateKernelLaunch`
 ///     - ::UR_RESULT_ERROR_UNSUPPORTED_FEATURE
-///         + If update functionality is not supported by the device.
+///         + If ::UR_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_KERNEL_ARGUMENTS is not supported by the device, but any of `pUpdateKernelLaunch->numNewMemObjArgs`, `pUpdateKernelLaunch->numNewPointerArgs`, or `pUpdateKernelLaunch->numNewValueArgs` are not zero.
+///         + If ::UR_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_LOCAL_WORK_SIZE is not supported by the device but `pUpdateKernelLaunch->pNewLocalWorkSize` is not nullptr.
+///         + If ::UR_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_LOCAL_WORK_SIZE is not supported by the device but `pUpdateKernelLaunch->pNewLocalWorkSize` is nullptr and `pUpdateKernelLaunch->pNewGlobalWorkSize` is not nullptr.
+///         + If ::UR_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_GLOBAL_WORK_SIZE is not supported by the device but `pUpdateKernelLaunch->pNewGlobalWorkSize` is not nullptr
+///         + If ::UR_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_GLOBAL_WORK_OFFSET is not supported by the device but `pUpdateKernelLaunch->pNewGlobalWorkOffset` is not nullptr.
+///         + If ::UR_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_KERNEL_HANDLE is not supported by the device but `pUpdateKernelLaunch->hNewKernel` is not nullptr.
 ///     - ::UR_RESULT_ERROR_INVALID_OPERATION
 ///         + If ::ur_exp_command_buffer_desc_t::isUpdatable was not set to true on creation of the command buffer `hCommand` belongs to.
 ///         + If the command-buffer `hCommand` belongs to has not been finalized.
-///     - ::UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_COMMAND_HANDLE_EXP - "If `hCommand` is not a kernel execution command."
+///     - ::UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_COMMAND_HANDLE_EXP
+///         + If `hCommand` is not a kernel execution command.
 ///     - ::UR_RESULT_ERROR_INVALID_MEM_OBJECT
 ///     - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGUMENT_INDEX
 ///     - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGUMENT_SIZE
@@ -9329,7 +9335,7 @@ urCommandBufferReleaseCommandExp(
 ///     - ::UR_RESULT_ERROR_INVALID_WORK_GROUP_SIZE
 ///     - ::UR_RESULT_ERROR_INVALID_VALUE
 ///         + If `pUpdateKernelLaunch->hNewKernel` was not passed to the `hKernel` or `phKernelAlternatives` parameters of ::urCommandBufferAppendKernelLaunchExp when this command was created.
-///         + If `pUpdateKernelLaunch->newWorkDim` is different from the current workDim in `hCommand` and, pUpdateKernelLaunch->pNewGlobalWorkSize, or pUpdateKernelLaunch->pNewGlobalWorkOffset are nullptr.
+///         + If `pUpdateKernelLaunch->newWorkDim` is different from the current workDim in `hCommand` and, `pUpdateKernelLaunch->pNewGlobalWorkSize`, or `pUpdateKernelLaunch->pNewGlobalWorkOffset` are nullptr.
 ///     - ::UR_RESULT_ERROR_OUT_OF_HOST_MEMORY
 ///     - ::UR_RESULT_ERROR_OUT_OF_RESOURCES
 UR_APIEXPORT ur_result_t UR_APICALL
@@ -9355,7 +9361,7 @@ urCommandBufferUpdateKernelLaunchExp(
 ///     - ::UR_RESULT_ERROR_INVALID_NULL_POINTER
 ///         + `NULL == phSignalEvent`
 ///     - ::UR_RESULT_ERROR_UNSUPPORTED_FEATURE
-///         + If UR_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_EVENTS is not supported by the device associated with `hCommand`.
+///         + If ::UR_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_EVENTS is not supported by the device associated with `hCommand`.
 ///     - ::UR_RESULT_ERROR_INVALID_OPERATION
 ///         + If ::ur_exp_command_buffer_desc_t::isUpdatable was not set to true on creation of the command buffer `hCommand` belongs to.
 ///         + If the command-buffer `hCommand` belongs to has not been finalized.
@@ -9382,7 +9388,7 @@ urCommandBufferUpdateSignalEventExp(
 ///     - ::UR_RESULT_ERROR_INVALID_NULL_HANDLE
 ///         + `NULL == hCommand`
 ///     - ::UR_RESULT_ERROR_UNSUPPORTED_FEATURE
-///         + If UR_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_EVENTS is not supported by the device associated with `hCommand`.
+///         + If ::UR_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_EVENTS is not supported by the device associated with `hCommand`.
 ///     - ::UR_RESULT_ERROR_INVALID_OPERATION
 ///         + If ::ur_exp_command_buffer_desc_t::isUpdatable was not set to true on creation of the command buffer `hCommand` belongs to.
 ///         + If the command-buffer `hCommand` belongs to has not been finalized.

scripts/core/exp-command-buffer.yml

@@ -289,8 +289,9 @@ params:
 returns:
     - $X_RESULT_ERROR_INVALID_CONTEXT
     - $X_RESULT_ERROR_INVALID_DEVICE
-    - $X_RESULT_ERROR_INVALID_OPERATION:
-        - "If `pCommandBufferDesc->isUpdatable` is true and `hDevice` does not support UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_SUPPORT_EXP."
+    - $X_RESULT_ERROR_UNSUPPORTED_FEATURE:
+        - "If `pCommandBufferDesc->isUpdatable` is true and `hDevice` returns 0
+        for the $X_DEVICE_INFO_COMMAND_BUFFER_UPDATE_CAPABILITIES_EXP query."
     - $X_RESULT_ERROR_OUT_OF_HOST_MEMORY
     - $X_RESULT_ERROR_OUT_OF_RESOURCES
 --- #--------------------------------------------------------------------------
@@ -1203,11 +1204,16 @@ params:
       desc: "[in] Struct defining how the kernel command is to be updated."
 returns:
     - $X_RESULT_ERROR_UNSUPPORTED_FEATURE:
-        - "If update functionality is not supported by the device."
+        - "If $X_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_KERNEL_ARGUMENTS is not supported by the device, but any of `pUpdateKernelLaunch->numNewMemObjArgs`, `pUpdateKernelLaunch->numNewPointerArgs`, or `pUpdateKernelLaunch->numNewValueArgs` are not zero."
+        - "If $X_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_LOCAL_WORK_SIZE is not supported by the device but `pUpdateKernelLaunch->pNewLocalWorkSize` is not nullptr."
+        - "If $X_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_LOCAL_WORK_SIZE is not supported by the device but `pUpdateKernelLaunch->pNewLocalWorkSize` is nullptr and `pUpdateKernelLaunch->pNewGlobalWorkSize` is not nullptr."
+        - "If $X_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_GLOBAL_WORK_SIZE is not supported by the device but `pUpdateKernelLaunch->pNewGlobalWorkSize` is not nullptr"
+        - "If $X_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_GLOBAL_WORK_OFFSET is not supported by the device but `pUpdateKernelLaunch->pNewGlobalWorkOffset` is not nullptr."
+        - "If $X_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_KERNEL_HANDLE is not supported by the device but `pUpdateKernelLaunch->hNewKernel` is not nullptr."
     - $X_RESULT_ERROR_INVALID_OPERATION:
         - "If $x_exp_command_buffer_desc_t::isUpdatable was not set to true on creation of the command buffer `hCommand` belongs to."
         - "If the command-buffer `hCommand` belongs to has not been finalized."
-    - $X_RESULT_ERROR_INVALID_COMMAND_BUFFER_COMMAND_HANDLE_EXP
+    - $X_RESULT_ERROR_INVALID_COMMAND_BUFFER_COMMAND_HANDLE_EXP:
         - "If `hCommand` is not a kernel execution command."
     - $X_RESULT_ERROR_INVALID_MEM_OBJECT
     - $X_RESULT_ERROR_INVALID_KERNEL_ARGUMENT_INDEX
@@ -1218,7 +1224,7 @@ returns:
     - $X_RESULT_ERROR_INVALID_WORK_GROUP_SIZE
     - $X_RESULT_ERROR_INVALID_VALUE:
         - "If `pUpdateKernelLaunch->hNewKernel` was not passed to the `hKernel` or `phKernelAlternatives` parameters of $xCommandBufferAppendKernelLaunchExp when this command was created."
-        - "If `pUpdateKernelLaunch->newWorkDim` is different from the current workDim in `hCommand` and, pUpdateKernelLaunch->pNewGlobalWorkSize, or pUpdateKernelLaunch->pNewGlobalWorkOffset are nullptr."
+        - "If `pUpdateKernelLaunch->newWorkDim` is different from the current workDim in `hCommand` and, `pUpdateKernelLaunch->pNewGlobalWorkSize`, or `pUpdateKernelLaunch->pNewGlobalWorkOffset` are nullptr."
     - $X_RESULT_ERROR_OUT_OF_HOST_MEMORY
     - $X_RESULT_ERROR_OUT_OF_RESOURCES
 --- #--------------------------------------------------------------------------
@@ -1236,7 +1242,7 @@ params:
       desc: "[out] Event to be signaled."
 returns:
     - $X_RESULT_ERROR_UNSUPPORTED_FEATURE:
-        - "If UR_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_EVENTS is not supported by the device associated with `hCommand`."
+        - "If $X_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_EVENTS is not supported by the device associated with `hCommand`."
     - $X_RESULT_ERROR_INVALID_OPERATION:
         - "If $x_exp_command_buffer_desc_t::isUpdatable was not set to true on creation of the command buffer `hCommand` belongs to."
         - "If the command-buffer `hCommand` belongs to has not been finalized."
@@ -1262,7 +1268,7 @@ params:
       desc: "[in][optional][range(0, numEventsInWaitList)] pointer to a list of events that must be complete before the command execution. If nullptr, the numEventsInWaitList must be 0, indicating no wait events."
 returns:
     - $X_RESULT_ERROR_UNSUPPORTED_FEATURE:
-        - "If UR_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_EVENTS is not supported by the device associated with `hCommand`."
+        - "If $X_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_EVENTS is not supported by the device associated with `hCommand`."
     - $X_RESULT_ERROR_INVALID_OPERATION:
         - "If $x_exp_command_buffer_desc_t::isUpdatable was not set to true on creation of the command buffer `hCommand` belongs to."
         - "If the command-buffer `hCommand` belongs to has not been finalized."

source/adapters/opencl/command_buffer.cpp

@@ -78,7 +78,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferCreateExp(
   bool DeviceSupportsUpdate = UpdateCapabilities > 0;
 
   if (IsUpdatable && !DeviceSupportsUpdate) {
-    return UR_RESULT_ERROR_INVALID_OPERATION;
+    return UR_RESULT_ERROR_UNSUPPORTED_FEATURE;
   }
 
   cl_command_buffer_properties_khr Properties[3] = {
@@ -92,7 +92,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferCreateExp(
 
   try {
     auto URCommandBuffer = std::make_unique<ur_exp_command_buffer_handle_t_>(
-        Queue, hContext, CLCommandBuffer, IsUpdatable);
+        Queue, hContext, hDevice, CLCommandBuffer, IsUpdatable);
     *phCommandBuffer = URCommandBuffer.release();
   } catch (...) {
     return UR_RESULT_ERROR_OUT_OF_RESOURCES;
@@ -540,18 +540,72 @@ void updateKernelArgs(std::vector<cl_mutable_dispatch_arg_khr> &CLArgs,
   }
 }
 
+ur_result_t validateCommandDesc(
+    ur_exp_command_buffer_command_handle_t Command,
+    const ur_exp_command_buffer_update_kernel_launch_desc_t *UpdateDesc) {
+  // Kernel handle updates are not yet supported.
+  if (UpdateDesc->hNewKernel && UpdateDesc->hNewKernel != Command->Kernel) {
+    return UR_RESULT_ERROR_UNSUPPORTED_FEATURE;
+  }
+
+  // Error if work-dim has changed but a new global size/offset hasn't been set
+  if (UpdateDesc->newWorkDim != Command->WorkDim &&
+      (!UpdateDesc->pNewGlobalWorkOffset || !UpdateDesc->pNewGlobalWorkSize)) {
+    return UR_RESULT_ERROR_INVALID_OPERATION;
+  }
+
+  // Verify that the device supports updating the aspects of the kernel that
+  // the user is requesting.
+  ur_device_handle_t URDevice = Command->hCommandBuffer->hDevice;
+  cl_device_id CLDevice = cl_adapter::cast<cl_device_id>(URDevice);
+
+  ur_device_command_buffer_update_capability_flags_t UpdateCapabilities = 0;
+  CL_RETURN_ON_FAILURE(
+      getDeviceCommandBufferUpdateCapabilities(CLDevice, UpdateCapabilities));
+
+  size_t *NewGlobalWorkOffset = UpdateDesc->pNewGlobalWorkOffset;
+  UR_ASSERT(
+      !NewGlobalWorkOffset ||
+          (UpdateCapabilities &
+           UR_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_GLOBAL_WORK_OFFSET),
+      UR_RESULT_ERROR_UNSUPPORTED_FEATURE);
+
+  size_t *NewLocalWorkSize = UpdateDesc->pNewLocalWorkSize;
+  UR_ASSERT(
+      !NewLocalWorkSize ||
+          (UpdateCapabilities &
+           UR_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_LOCAL_WORK_SIZE),
+      UR_RESULT_ERROR_UNSUPPORTED_FEATURE);
+
+  size_t *NewGlobalWorkSize = UpdateDesc->pNewGlobalWorkSize;
+  UR_ASSERT(
+      !NewGlobalWorkSize ||
+          (UpdateCapabilities &
+           UR_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_GLOBAL_WORK_SIZE),
+      UR_RESULT_ERROR_UNSUPPORTED_FEATURE);
+  UR_ASSERT(
+      !(NewGlobalWorkSize && !NewLocalWorkSize) ||
+          (UpdateCapabilities &
+           UR_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_LOCAL_WORK_SIZE),
+      UR_RESULT_ERROR_UNSUPPORTED_FEATURE);
+
+  UR_ASSERT(
+      (!UpdateDesc->numNewMemObjArgs && !UpdateDesc->numNewPointerArgs &&
+       !UpdateDesc->numNewValueArgs) ||
+          (UpdateCapabilities &
+           UR_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_KERNEL_ARGUMENTS),
+      UR_RESULT_ERROR_UNSUPPORTED_FEATURE);
+
+  return UR_RESULT_SUCCESS;
+}
 } // end anonymous namespace
 
 UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp(
     ur_exp_command_buffer_command_handle_t hCommand,
     const ur_exp_command_buffer_update_kernel_launch_desc_t
         *pUpdateKernelLaunch) {
 
-  // Kernel handle updates are not yet supported.
-  if (pUpdateKernelLaunch->hNewKernel &&
-      pUpdateKernelLaunch->hNewKernel != hCommand->Kernel) {
-    return UR_RESULT_ERROR_UNSUPPORTED_FEATURE;
-  }
+  UR_RETURN_ON_FAILURE(validateCommandDesc(hCommand, pUpdateKernelLaunch));
 
   ur_exp_command_buffer_handle_t hCommandBuffer = hCommand->hCommandBuffer;
   cl_context CLContext = cl_adapter::cast<cl_context>(hCommandBuffer->hContext);
@@ -565,12 +619,6 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp(
   if (!hCommandBuffer->IsFinalized || !hCommandBuffer->IsUpdatable)
     return UR_RESULT_ERROR_INVALID_OPERATION;
 
-  if (pUpdateKernelLaunch->newWorkDim != hCommand->WorkDim &&
-      (!pUpdateKernelLaunch->pNewGlobalWorkOffset ||
-       !pUpdateKernelLaunch->pNewGlobalWorkSize)) {
-    return UR_RESULT_ERROR_INVALID_OPERATION;
-  }
-
   // Find the CL USM pointer arguments to the kernel to update
   std::vector<cl_mutable_dispatch_arg_khr> CLUSMArgs;
   updateKernelPointerArgs(CLUSMArgs, pUpdateKernelLaunch);

source/adapters/opencl/command_buffer.hpp

@@ -64,6 +64,8 @@ struct ur_exp_command_buffer_handle_t_ {
   ur_queue_handle_t hInternalQueue;
   /// Context the command-buffer is created for.
   ur_context_handle_t hContext;
+  /// Device the command-buffer is created for.
+  ur_device_handle_t hDevice;
   /// OpenCL command-buffer object.
   cl_command_buffer_khr CLCommandBuffer;
   /// Set to true if the kernel commands in the command-buffer can be updated,
@@ -83,9 +85,10 @@ struct ur_exp_command_buffer_handle_t_ {
 
   ur_exp_command_buffer_handle_t_(ur_queue_handle_t hQueue,
                                   ur_context_handle_t hContext,
+                                  ur_device_handle_t hDevice,
                                   cl_command_buffer_khr CLCommandBuffer,
                                   bool IsUpdatable)
-      : hInternalQueue(hQueue), hContext(hContext),
+      : hInternalQueue(hQueue), hContext(hContext), hDevice(hDevice),
         CLCommandBuffer(CLCommandBuffer), IsUpdatable(IsUpdatable),
         IsFinalized(false), RefCountInternal(0), RefCountExternal(0) {}