Make newWorkDim non-optional and remove newLocalWorkgroup nullptr errors

oneapi-src · Sep 10, 2024 · f478846 · f478846
1 parent fb923ef
commit f478846
Show file tree

Hide file tree

Showing 26 changed files with 314 additions and 374 deletions.
diff --git a/include/ur_api.h b/include/ur_api.h
@@ -8286,10 +8286,8 @@ typedef struct ur_exp_command_buffer_update_kernel_launch_desc_t {
                                                                                ///< values that describe the number of global work-items.
     size_t *pNewLocalWorkSize;                                                 ///< [in][optional][range(0, newWorkDim)] Array of newWorkDim unsigned
                                                                                ///< values that describe the number of work-items that make up a
-                                                                               ///< work-group. If newWorkDim is non-zero and pNewLocalWorkSize is
-                                                                               ///< nullptr, then runtime implementation will choose the work-group size.
-                                                                               ///< If newWorkDim is zero and pNewLocalWorkSize is nullptr, then the local
-                                                                               ///< work size is unchanged.
+                                                                               ///< work-group. If pNewLocalWorkSize is nullptr, then the local work size
+                                                                               ///< is unchanged.
 
 } ur_exp_command_buffer_update_kernel_launch_desc_t;
 
@@ -8427,7 +8425,9 @@ urCommandBufferAppendKernelLaunchExp(
     uint32_t workDim,                                             ///< [in] Dimension of the kernel execution.
     const size_t *pGlobalWorkOffset,                              ///< [in] Offset to use when executing kernel.
     const size_t *pGlobalWorkSize,                                ///< [in] Global work size to use when executing kernel.
-    const size_t *pLocalWorkSize,                                 ///< [in][optional] Local work size to use when executing kernel.
+    const size_t *pLocalWorkSize,                                 ///< [in][optional] Local work size to use when executing kernel. If this
+                                                                  ///< parameter is nullptr, then a local work size will be generated by the
+                                                                  ///< implementation.
     uint32_t numKernelAlternatives,                               ///< [in] The number of kernel alternatives provided in
                                                                   ///< phKernelAlternatives.
     ur_kernel_handle_t *phKernelAlternatives,                     ///< [in][optional][range(0, numKernelAlternatives)] List of kernels
@@ -8950,17 +8950,15 @@ urCommandBufferReleaseCommandExp(
 ///     - ::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.
-///         + If `pUpdateKernellaunch->hNewKernel` is different from the currently active kernel in `hCommand`, and `pUpdateKernellaunch->newWorkDim` is zero.
-///         + If `pUpdateKernellaunch->hNewKernel` is equal to the currently active kernel in `hCommand`, and `pUpdateKernellaunch->newWorkDim` is non-zero and different from the work-dim currently associated with `hCommand`.
-///         + If `pUpdateKernellaunch->newWorkDim` is non-zero, and `pUpdateKernelLaunch->pNewLocalWorkSize` is set to a non-NULL value, and `pUpdateKernelLaunch->pNewGlobalWorkSize` is NULL.
-///         + If `pUpdateKernellaunch->hNewKernel` is equal to the current kernel associated with `hCommand`, and `pUpdateKernellaunch->newWorkDim` is non-zero, and `pUpdateKernelLaunch->pNewLocalWorkSize` is set to a non-NULL value while `hCommand` is currently associated with a NULL local work size.
-///         + If `pUpdateKernellaunch->hNewKernel` is equal to the current kernel associated with `hCommand`, and `pUpdateKernellaunch->newWorkDim` is non-zero, and `pUpdateKernelLaunch->pNewLocalWorkSize` is set to a NULL value while `hCommand` is currently associated with a non-NULL local work size.
+///         + `pUpdateKernelLaunch->pNewLocalWorkSize != NULL && pUpdateKernelLaunch->pNewGlobalWorkSize == NULL`
+///         + If `pUpdateKernellaunch->hNewKernel` is equal to the currently active kernel in `hCommand`, and `pUpdateKernellaunch->newWorkDim` is different from the work-dim currently associated with `hCommand`.
 ///     - ::UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_COMMAND_HANDLE_EXP
 ///     - ::UR_RESULT_ERROR_INVALID_MEM_OBJECT
 ///     - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGUMENT_INDEX
 ///     - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGUMENT_SIZE
 ///     - ::UR_RESULT_ERROR_INVALID_ENUMERATION
 ///     - ::UR_RESULT_ERROR_INVALID_WORK_DIMENSION
+///         + `pUpdateKernelLaunch->newWorkDim < 0 || pUpdateKernelLaunch->newWorkDim > 3`
 ///     - ::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.

diff --git a/scripts/core/exp-command-buffer.yml b/scripts/core/exp-command-buffer.yml
@@ -226,7 +226,7 @@ members:
       desc: "[in][optional][range(0, newWorkDim)] Array of newWorkDim unsigned values that describe the number of global work-items."
     - type: "size_t*"
       name: pNewLocalWorkSize
-      desc: "[in][optional][range(0, newWorkDim)] Array of newWorkDim unsigned values that describe the number of work-items that make up a work-group. If newWorkDim is non-zero and pNewLocalWorkSize is nullptr, then runtime implementation will choose the work-group size. If newWorkDim is zero and pNewLocalWorkSize is nullptr, then the local work size is unchanged."
+      desc: "[in][optional][range(0, newWorkDim)] Array of newWorkDim unsigned values that describe the number of work-items that make up a work-group. If pNewLocalWorkSize is nullptr, then the local work size is unchanged."
 --- #--------------------------------------------------------------------------
 type: typedef
 desc: "A value that identifies a command inside of a command-buffer, used for defining dependencies between commands in the same command-buffer."
@@ -333,7 +333,7 @@ params:
       desc: "[in] Global work size to use when executing kernel."
     - type: "const size_t*"
       name: pLocalWorkSize
-      desc: "[in][optional] Local work size to use when executing kernel."
+      desc: "[in][optional] Local work size to use when executing kernel. If this parameter is nullptr, then a local work size will be generated by the implementation."
     - type: uint32_t
       name: "numKernelAlternatives"
       desc: "[in] The number of kernel alternatives provided in phKernelAlternatives."
@@ -954,17 +954,15 @@ returns:
     - $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."
-        - "If `pUpdateKernellaunch->hNewKernel` is different from the currently active kernel in `hCommand`, and `pUpdateKernellaunch->newWorkDim` is zero."
-        - "If `pUpdateKernellaunch->hNewKernel` is equal to the currently active kernel in `hCommand`, and `pUpdateKernellaunch->newWorkDim` is non-zero and different from the work-dim currently associated with `hCommand`."
-        - "If `pUpdateKernellaunch->newWorkDim` is non-zero, and `pUpdateKernelLaunch->pNewLocalWorkSize` is set to a non-NULL value, and `pUpdateKernelLaunch->pNewGlobalWorkSize` is NULL."
-        - "If `pUpdateKernellaunch->hNewKernel` is equal to the current kernel associated with `hCommand`, and `pUpdateKernellaunch->newWorkDim` is non-zero, and `pUpdateKernelLaunch->pNewLocalWorkSize` is set to a non-NULL value while `hCommand` is currently associated with a NULL local work size."
-        - "If `pUpdateKernellaunch->hNewKernel` is equal to the current kernel associated with `hCommand`, and `pUpdateKernellaunch->newWorkDim` is non-zero, and `pUpdateKernelLaunch->pNewLocalWorkSize` is set to a NULL value while `hCommand` is currently associated with a non-NULL local work size."
+        - "`pUpdateKernelLaunch->pNewLocalWorkSize != NULL && pUpdateKernelLaunch->pNewGlobalWorkSize == NULL`"
+        - "If `pUpdateKernellaunch->hNewKernel` is equal to the currently active kernel in `hCommand`, and `pUpdateKernellaunch->newWorkDim` is different from the work-dim currently associated with `hCommand`."
     - $X_RESULT_ERROR_INVALID_COMMAND_BUFFER_COMMAND_HANDLE_EXP
     - $X_RESULT_ERROR_INVALID_MEM_OBJECT
     - $X_RESULT_ERROR_INVALID_KERNEL_ARGUMENT_INDEX
     - $X_RESULT_ERROR_INVALID_KERNEL_ARGUMENT_SIZE
     - $X_RESULT_ERROR_INVALID_ENUMERATION
-    - $X_RESULT_ERROR_INVALID_WORK_DIMENSION
+    - $X_RESULT_ERROR_INVALID_WORK_DIMENSION:
+        - "`pUpdateKernelLaunch->newWorkDim < 0 || pUpdateKernelLaunch->newWorkDim > 3`"
     - $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."

diff --git a/source/adapters/cuda/command_buffer.cpp b/source/adapters/cuda/command_buffer.cpp
@@ -887,37 +887,11 @@ validateCommandDesc(ur_exp_command_buffer_command_handle_t Command,
     return UR_RESULT_ERROR_INVALID_OPERATION;
   }
 
-  const uint32_t NewWorkDim = UpdateCommandDesc->newWorkDim;
-  if (!NewWorkDim && Command->Kernel != UpdateCommandDesc->hNewKernel) {
+  if (UpdateCommandDesc->newWorkDim != Command->WorkDim &&
+      Command->Kernel == UpdateCommandDesc->hNewKernel) {
     return UR_RESULT_ERROR_INVALID_OPERATION;
   }
 
-  if (NewWorkDim) {
-    UR_ASSERT(NewWorkDim > 0, UR_RESULT_ERROR_INVALID_WORK_DIMENSION);
-    UR_ASSERT(NewWorkDim < 4, UR_RESULT_ERROR_INVALID_WORK_DIMENSION);
-
-    if (NewWorkDim != Command->WorkDim &&
-        Command->Kernel == UpdateCommandDesc->hNewKernel) {
-      return UR_RESULT_ERROR_INVALID_OPERATION;
-    }
-
-    // Error If Local size and not global size
-    if ((UpdateCommandDesc->pNewLocalWorkSize != nullptr) &&
-        (UpdateCommandDesc->pNewGlobalWorkSize == nullptr)) {
-      return UR_RESULT_ERROR_INVALID_OPERATION;
-    }
-
-    // Error if local size non-nullptr and created with null
-    // or if local size nullptr and created with non-null
-    const bool IsNewLocalSizeNull =
-        UpdateCommandDesc->pNewLocalWorkSize == nullptr;
-    const bool IsOriginalLocalSizeNull = Command->isNullLocalSize();
-
-    if (IsNewLocalSizeNull ^ IsOriginalLocalSizeNull) {
-      return UR_RESULT_ERROR_INVALID_OPERATION;
-    }
-  }
-
   if (!Command->ValidKernelHandles.count(UpdateCommandDesc->hNewKernel)) {
     return UR_RESULT_ERROR_INVALID_VALUE;
   }

diff --git a/source/adapters/cuda/device.cpp b/source/adapters/cuda/device.cpp
@@ -1093,7 +1093,6 @@ UR_APIEXPORT ur_result_t UR_APICALL urDeviceGetInfo(ur_device_handle_t hDevice,
     return UR_RESULT_ERROR_UNSUPPORTED_ENUMERATION;
 
   case UR_DEVICE_INFO_COMMAND_BUFFER_SUPPORT_EXP:
-    /*case UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_SUPPORT_EXP:*/
     return ReturnValue(true);
   case UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_CAPABILITIES_EXP: {
     ur_device_command_buffer_update_capability_flags_t UpdateCapabilities =

diff --git a/source/adapters/hip/command_buffer.cpp b/source/adapters/hip/command_buffer.cpp
@@ -48,9 +48,9 @@ commandHandleReleaseInternal(ur_exp_command_buffer_command_handle_t Command) {
 
 ur_exp_command_buffer_handle_t_::ur_exp_command_buffer_handle_t_(
     ur_context_handle_t hContext, ur_device_handle_t hDevice, bool IsUpdatable)
-    : Context(hContext), Device(hDevice), IsUpdatable(IsUpdatable),
-      HIPGraph{nullptr}, HIPGraphExec{nullptr}, RefCountInternal{1},
-      RefCountExternal{1}, NextSyncPoint{0} {
+    : Context(hContext), Device(hDevice),
+      IsUpdatable(IsUpdatable), HIPGraph{nullptr}, HIPGraphExec{nullptr},
+      RefCountInternal{1}, RefCountExternal{1}, NextSyncPoint{0} {
   urContextRetain(hContext);
   urDeviceRetain(hDevice);
 }
@@ -330,9 +330,15 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferAppendKernelLaunchExp(
             UR_RESULT_ERROR_INVALID_KERNEL);
   UR_ASSERT(workDim > 0, UR_RESULT_ERROR_INVALID_WORK_DIMENSION);
   UR_ASSERT(workDim < 4, UR_RESULT_ERROR_INVALID_WORK_DIMENSION);
+
   UR_ASSERT(!(pSyncPointWaitList == NULL && numSyncPointsInWaitList > 0),
             UR_RESULT_ERROR_INVALID_EVENT_WAIT_LIST);
 
+  for (uint32_t i = 0; i < numKernelAlternatives; ++i) {
+    UR_ASSERT(phKernelAlternatives[i] != hKernel,
+              UR_RESULT_ERROR_INVALID_VALUE);
+  }
+
   hipGraphNode_t GraphNode;
   std::vector<hipGraphNode_t> DepsList;
 
@@ -866,37 +872,11 @@ validateCommandDesc(ur_exp_command_buffer_command_handle_t Command,
     return UR_RESULT_ERROR_INVALID_OPERATION;
   }
 
-  const uint32_t NewWorkDim = UpdateCommandDesc->newWorkDim;
-  if (!NewWorkDim && Command->Kernel != UpdateCommandDesc->hNewKernel) {
+  if (UpdateCommandDesc->newWorkDim != Command->WorkDim &&
+      Command->Kernel == UpdateCommandDesc->hNewKernel) {
     return UR_RESULT_ERROR_INVALID_OPERATION;
   }
 
-  if (NewWorkDim) {
-    UR_ASSERT(NewWorkDim > 0, UR_RESULT_ERROR_INVALID_WORK_DIMENSION);
-    UR_ASSERT(NewWorkDim < 4, UR_RESULT_ERROR_INVALID_WORK_DIMENSION);
-
-    if (NewWorkDim != Command->WorkDim &&
-        Command->Kernel == UpdateCommandDesc->hNewKernel) {
-      return UR_RESULT_ERROR_INVALID_OPERATION;
-    }
-
-    // Error If Local size and not global size
-    if ((UpdateCommandDesc->pNewLocalWorkSize != nullptr) &&
-        (UpdateCommandDesc->pNewGlobalWorkSize == nullptr)) {
-      return UR_RESULT_ERROR_INVALID_OPERATION;
-    }
-
-    // Error if local size non-nullptr and created with null
-    // or if local size nullptr and created with non-null
-    const bool IsNewLocalSizeNull =
-        UpdateCommandDesc->pNewLocalWorkSize == nullptr;
-    const bool IsOriginalLocalSizeNull = Command->isNullLocalSize();
-
-    if (IsNewLocalSizeNull ^ IsOriginalLocalSizeNull) {
-      return UR_RESULT_ERROR_INVALID_OPERATION;
-    }
-  }
-
   if (!Command->ValidKernelHandles.count(UpdateCommandDesc->hNewKernel)) {
     return UR_RESULT_ERROR_INVALID_VALUE;
   }
@@ -907,8 +887,8 @@ validateCommandDesc(ur_exp_command_buffer_command_handle_t Command,
 /**
  * Updates the arguments of CommandDesc->hNewKernel
  * @param[in] Device The device associated with the kernel being updated.
- * @param[in] UpdateCommandDesc The update command description that contains the
- * new kernel and its arguments.
+ * @param[in] UpdateCommandDesc The update command description that contains
+ * the new kernel and its arguments.
  * @return UR_RESULT_SUCCESS or an error code on failure
  */
 ur_result_t
@@ -1020,8 +1000,8 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp(
       updateKernelArguments(CommandBuffer->Device, pUpdateKernelLaunch));
   UR_CHECK_ERROR(updateCommand(hCommand, pUpdateKernelLaunch));
 
-  // If no worksize is provided make sure we pass nullptr to setKernelParams so
-  // it can guess the local work size.
+  // If no worksize is provided make sure we pass nullptr to setKernelParams
+  // so it can guess the local work size.
   const bool ProvidedLocalSize = !hCommand->isNullLocalSize();
   size_t *LocalWorkSize = ProvidedLocalSize ? hCommand->LocalWorkSize : nullptr;
 

diff --git a/source/adapters/hip/device.cpp b/source/adapters/hip/device.cpp
@@ -903,20 +903,18 @@ UR_APIEXPORT ur_result_t UR_APICALL urDeviceGetInfo(ur_device_handle_t hDevice,
   case UR_DEVICE_INFO_IL_VERSION:
   case UR_DEVICE_INFO_ASYNC_BARRIER:
     return UR_RESULT_ERROR_UNSUPPORTED_ENUMERATION;
+  case UR_DEVICE_INFO_COMMAND_BUFFER_SUPPORT_EXP: {
+    int DriverVersion = 0;
+    UR_CHECK_ERROR(hipDriverGetVersion(&DriverVersion));
 
-  case UR_DEVICE_INFO_COMMAND_BUFFER_SUPPORT_EXP:
-    /*case UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_SUPPORT_EXP: */ {
-      int DriverVersion = 0;
-      UR_CHECK_ERROR(hipDriverGetVersion(&DriverVersion));
-
-      // Return supported for the UR command-buffer experimental feature on
-      // ROCM 5.5.1 and later. This is to workaround HIP driver bug
-      // https://github.com/ROCm/HIP/issues/2450 in older versions.
-      //
-      // The version is returned as (10000000 major + 1000000 minor + patch).
-      const int CmdBufDriverMinVersion = 50530202; // ROCM 5.5.1
-      return ReturnValue(DriverVersion >= CmdBufDriverMinVersion);
-    }
+    // Return supported for the UR command-buffer experimental feature on
+    // ROCM 5.5.1 and later. This is to workaround HIP driver bug
+    // https://github.com/ROCm/HIP/issues/2450 in older versions.
+    //
+    // The version is returned as (10000000 major + 1000000 minor + patch).
+    const int CmdBufDriverMinVersion = 50530202; // ROCM 5.5.1
+    return ReturnValue(DriverVersion >= CmdBufDriverMinVersion);
+  }
   case UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_CAPABILITIES_EXP: {
     int DriverVersion = 0;
     UR_CHECK_ERROR(hipDriverGetVersion(&DriverVersion));

diff --git a/source/adapters/level_zero/command_buffer.cpp b/source/adapters/level_zero/command_buffer.cpp
@@ -1320,35 +1320,23 @@ ur_result_t validateCommandDesc(
           ->mutableCommandFlags;
   logger::debug("Mutable features supported by device {}", SupportedFeatures);
 
-  uint32_t Dim = CommandDesc->newWorkDim;
-  if (Dim != 0) {
-    // Error if work dim changes
-    if (Dim != Command->WorkDim) {
-      return UR_RESULT_ERROR_INVALID_OPERATION;
-    }
-
-    // Error If Local size and not global size
-    if ((CommandDesc->pNewLocalWorkSize != nullptr) &&
-        (CommandDesc->pNewGlobalWorkSize == nullptr)) {
-      return UR_RESULT_ERROR_INVALID_OPERATION;
-    }
-
-    // Error if local size non-nullptr and created with null
-    // or if local size nullptr and created with non-null
-    const bool IsNewLocalSizeNull = CommandDesc->pNewLocalWorkSize == nullptr;
-    const bool IsOriginalLocalSizeNull = !Command->UserDefinedLocalSize;
+  // Kernel handle updates are not yet supported.
+  if (CommandDesc->hNewKernel != Command->Kernel) {
+    return UR_RESULT_ERROR_UNSUPPORTED_FEATURE;
+  }
 
-    if (IsNewLocalSizeNull ^ IsOriginalLocalSizeNull) {
-      return UR_RESULT_ERROR_INVALID_OPERATION;
-    }
+  // Error if work dim changes
+  if (CommandDesc->hNewKernel == Command->Kernel &&
+      CommandDesc->newWorkDim != Command->WorkDim) {
+    return UR_RESULT_ERROR_INVALID_OPERATION;
   }
 
   // Check if new global offset is provided.
   size_t *NewGlobalWorkOffset = CommandDesc->pNewGlobalWorkOffset;
   UR_ASSERT(!NewGlobalWorkOffset ||
                 (SupportedFeatures & ZE_MUTABLE_COMMAND_EXP_FLAG_GLOBAL_OFFSET),
             UR_RESULT_ERROR_UNSUPPORTED_FEATURE);
-  if (NewGlobalWorkOffset && Dim > 0) {
+  if (NewGlobalWorkOffset) {
     if (!CommandBuffer->Context->getPlatform()
              ->ZeDriverGlobalOffsetExtensionFound) {
       logger::error("No global offset extension found on this driver");
@@ -1618,8 +1606,6 @@ ur_result_t urCommandBufferUpdateKernelLaunchExp(
     ur_exp_command_buffer_command_handle_t Command,
     const ur_exp_command_buffer_update_kernel_launch_desc_t *CommandDesc) {
   UR_ASSERT(Command->Kernel, UR_RESULT_ERROR_INVALID_NULL_HANDLE);
-  UR_ASSERT(CommandDesc->newWorkDim <= 3,
-            UR_RESULT_ERROR_INVALID_WORK_DIMENSION);
 
   // Lock command, kernel and command buffer for update.
   std::scoped_lock<ur_shared_mutex, ur_shared_mutex, ur_shared_mutex> Guard(