[EXP][CMDBUF] Make command handle behaviour consistent

- Attempting to obtain a command handle when the command buffer is not updatable is now an error across all adapters - command-buffer spec and adapter code update to reflect this - invalid_update CTS test updated to reflect new behaviour - Add ability to query command-buffer descriptor properties
oneapi-src · Oct 7, 2024 · bee945e · bee945e
1 parent df6da35
commit bee945e
Show file tree

Hide file tree

Showing 15 changed files with 123 additions and 28 deletions.
diff --git a/include/ur_api.h b/include/ur_api.h
@@ -8196,6 +8196,10 @@ typedef enum ur_exp_command_buffer_info_t {
                                                     ///< The reference count returned should be considered immediately stale.
                                                     ///< It is unsuitable for general use in applications. This feature is
                                                     ///< provided for identifying memory leaks.
+    UR_EXP_COMMAND_BUFFER_INFO_DESCRIPTOR = 1,      ///< [::ur_exp_command_buffer_desc_t] Returns a ::ur_exp_command_buffer_desc_t
+                                                    ///< with the properties of the command-buffer. Returned values may differ
+                                                    ///< from those passed on construction if the property was ignored by the
+                                                    ///< adapter.
     /// @cond
     UR_EXP_COMMAND_BUFFER_INFO_FORCE_UINT32 = 0x7fffffff
     /// @endcond
@@ -8445,6 +8449,7 @@ urCommandBufferFinalizeExp(
 ///         + `pSyncPointWaitList != NULL && numSyncPointsInWaitList == 0`
 ///     - ::UR_RESULT_ERROR_OUT_OF_HOST_MEMORY
 ///     - ::UR_RESULT_ERROR_OUT_OF_RESOURCES
+///     - ::UR_RESULT_ERROR_INVALID_OPERATION - "phCommand is not NULL and hCommandBuffer is not updatable."
 UR_APIEXPORT ur_result_t UR_APICALL
 urCommandBufferAppendKernelLaunchExp(
     ur_exp_command_buffer_handle_t hCommandBuffer,                ///< [in] Handle of the command-buffer object.
@@ -8466,7 +8471,8 @@ urCommandBufferAppendKernelLaunchExp(
     const ur_exp_command_buffer_sync_point_t *pSyncPointWaitList, ///< [in][optional] A list of sync points that this command depends on. May
                                                                   ///< be ignored if command-buffer is in-order.
     ur_exp_command_buffer_sync_point_t *pSyncPoint,               ///< [out][optional] Sync point associated with this command.
-    ur_exp_command_buffer_command_handle_t *phCommand             ///< [out][optional] Handle to this command.
+    ur_exp_command_buffer_command_handle_t *phCommand             ///< [out][optional] Handle to this command. Only available if the
+                                                                  ///< command-buffer is updatable.
 );
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -9006,7 +9012,7 @@ urCommandBufferUpdateKernelLaunchExp(
 ///     - ::UR_RESULT_ERROR_INVALID_NULL_HANDLE
 ///         + `NULL == hCommandBuffer`
 ///     - ::UR_RESULT_ERROR_INVALID_ENUMERATION
-///         + `::UR_EXP_COMMAND_BUFFER_INFO_REFERENCE_COUNT < propName`
+///         + `::UR_EXP_COMMAND_BUFFER_INFO_DESCRIPTOR < propName`
 ///     - ::UR_RESULT_ERROR_UNSUPPORTED_ENUMERATION
 ///         + If `propName` is not supported by the adapter.
 ///     - ::UR_RESULT_ERROR_INVALID_SIZE

diff --git a/include/ur_print.hpp b/include/ur_print.hpp
@@ -9810,6 +9810,9 @@ inline std::ostream &operator<<(std::ostream &os, enum ur_exp_command_buffer_inf
     case UR_EXP_COMMAND_BUFFER_INFO_REFERENCE_COUNT:
         os << "UR_EXP_COMMAND_BUFFER_INFO_REFERENCE_COUNT";
         break;
+    case UR_EXP_COMMAND_BUFFER_INFO_DESCRIPTOR:
+        os << "UR_EXP_COMMAND_BUFFER_INFO_DESCRIPTOR";
+        break;
     default:
         os << "unknown enumerator";
         break;
@@ -9838,6 +9841,18 @@ inline ur_result_t printTagged(std::ostream &os, const void *ptr, ur_exp_command
 
         os << ")";
     } break;
+    case UR_EXP_COMMAND_BUFFER_INFO_DESCRIPTOR: {
+        const ur_exp_command_buffer_desc_t *tptr = (const ur_exp_command_buffer_desc_t *)ptr;
+        if (sizeof(ur_exp_command_buffer_desc_t) > size) {
+            os << "invalid size (is: " << size << ", expected: >=" << sizeof(ur_exp_command_buffer_desc_t) << ")";
+            return UR_RESULT_ERROR_INVALID_SIZE;
+        }
+        os << (const void *)(tptr) << " (";
+
+        os << *tptr;
+
+        os << ")";
+    } break;
     default:
         os << "unknown enumerator";
         return UR_RESULT_ERROR_INVALID_ENUMERATION;

diff --git a/scripts/core/EXP-COMMAND-BUFFER.rst b/scripts/core/EXP-COMMAND-BUFFER.rst
@@ -301,6 +301,7 @@ Enums
     * ${X}_FUNCTION_COMMAND_BUFFER_UPDATE_KERNEL_LAUNCH_EXP
 * ${x}_exp_command_buffer_info_t
     * ${X}_EXP_COMMAND_BUFFER_INFO_REFERENCE_COUNT
+    * ${X}_EXP_COMMAND_BUFFER_INFO_DESCRIPTOR
 * ${x}_exp_command_buffer_command_info_t
     * ${X}_EXP_COMMAND_BUFFER_COMMAND_INFO_REFERENCE_COUNT
 

diff --git a/scripts/core/exp-command-buffer.yml b/scripts/core/exp-command-buffer.yml
@@ -106,6 +106,12 @@ etors:
             [uint32_t] Reference count of the command-buffer object.
             The reference count returned should be considered immediately stale.
             It is unsuitable for general use in applications. This feature is provided for identifying memory leaks.
+    - name: DESCRIPTOR
+      desc: |
+            [$x_exp_command_buffer_desc_t] Returns a $x_exp_command_buffer_desc_t
+            with the properties of the command-buffer. Returned values may differ
+            from those passed on construction if the property was ignored by the
+            adapter.
 --- #--------------------------------------------------------------------------
 type: enum
 desc: "Command-buffer command query information type"
@@ -366,7 +372,8 @@ params:
       desc: "[out][optional] Sync point associated with this command."
     - type: "$x_exp_command_buffer_command_handle_t*"
       name: phCommand
-      desc: "[out][optional] Handle to this command."
+      desc: "[out][optional] Handle to this command. Only available if the
+      command-buffer is updatable."
 returns:
     - $X_RESULT_ERROR_INVALID_COMMAND_BUFFER_EXP
     - $X_RESULT_ERROR_INVALID_KERNEL
@@ -382,6 +389,8 @@ returns:
         - "`pSyncPointWaitList != NULL && numSyncPointsInWaitList == 0`"
     - $X_RESULT_ERROR_OUT_OF_HOST_MEMORY
     - $X_RESULT_ERROR_OUT_OF_RESOURCES
+    - $X_RESULT_ERROR_INVALID_OPERATION
+        - "phCommand is not NULL and hCommandBuffer is not updatable."
 --- #--------------------------------------------------------------------------
 type: function
 desc: "Append a USM memcpy command to a command-buffer object."

diff --git a/source/adapters/cuda/command_buffer.cpp b/source/adapters/cuda/command_buffer.cpp
@@ -359,6 +359,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferAppendKernelLaunchExp(
     ur_exp_command_buffer_sync_point_t *pSyncPoint,
     ur_exp_command_buffer_command_handle_t *phCommand) {
   // Preconditions
+  // Command handles can only be obtained from updatable command-buffers
+  UR_ASSERT(!(phCommand && !hCommandBuffer->IsUpdatable),
+            UR_RESULT_ERROR_INVALID_OPERATION);
   UR_ASSERT(hCommandBuffer->Context == hKernel->getContext(),
             UR_RESULT_ERROR_INVALID_KERNEL);
   UR_ASSERT(workDim > 0, UR_RESULT_ERROR_INVALID_WORK_DIMENSION);
@@ -1064,6 +1067,16 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferGetInfoExp(
   switch (propName) {
   case UR_EXP_COMMAND_BUFFER_INFO_REFERENCE_COUNT:
     return ReturnValue(hCommandBuffer->getExternalReferenceCount());
+  case UR_EXP_COMMAND_BUFFER_INFO_DESCRIPTOR: {
+    ur_exp_command_buffer_desc_t Descriptor{};
+    Descriptor.stype = UR_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_DESC;
+    Descriptor.pNext = nullptr;
+    Descriptor.isUpdatable = hCommandBuffer->IsUpdatable;
+    Descriptor.isInOrder = false;
+    Descriptor.enableProfiling = false;
+
+    return ReturnValue(Descriptor);
+  }
   default:
     assert(!"Command-buffer info request not implemented");
   }

diff --git a/source/adapters/hip/command_buffer.cpp b/source/adapters/hip/command_buffer.cpp
@@ -326,6 +326,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferAppendKernelLaunchExp(
     ur_exp_command_buffer_sync_point_t *pSyncPoint,
     ur_exp_command_buffer_command_handle_t *phCommand) {
   // Preconditions
+  // Command handles can only be obtained from updatable command-buffers
+  UR_ASSERT(!(phCommand && !hCommandBuffer->IsUpdatable),
+            UR_RESULT_ERROR_INVALID_OPERATION);
   UR_ASSERT(hCommandBuffer->Context == hKernel->getContext(),
             UR_RESULT_ERROR_INVALID_KERNEL);
   UR_ASSERT(workDim > 0, UR_RESULT_ERROR_INVALID_WORK_DIMENSION);
@@ -1041,6 +1044,15 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferGetInfoExp(
   switch (propName) {
   case UR_EXP_COMMAND_BUFFER_INFO_REFERENCE_COUNT:
     return ReturnValue(hCommandBuffer->getExternalReferenceCount());
+  case UR_EXP_COMMAND_BUFFER_INFO_DESCRIPTOR: {
+    ur_exp_command_buffer_desc_t Descriptor{};
+    Descriptor.stype = UR_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_DESC;
+    Descriptor.pNext = nullptr;
+    Descriptor.isUpdatable = hCommandBuffer->IsUpdatable;
+    Descriptor.isInOrder = false, Descriptor.enableProfiling = false;
+
+    return ReturnValue(Descriptor);
+  }
   default:
     assert(!"Command-buffer info request not implemented");
   }

diff --git a/source/adapters/level_zero/command_buffer.cpp b/source/adapters/level_zero/command_buffer.cpp
@@ -740,6 +740,9 @@ ur_result_t urCommandBufferAppendKernelLaunchExp(
     ur_exp_command_buffer_sync_point_t *RetSyncPoint,
     ur_exp_command_buffer_command_handle_t *Command) {
   UR_ASSERT(Kernel->Program, UR_RESULT_ERROR_INVALID_NULL_POINTER);
+  // Command handles can only be obtained from updatable command-buffers
+  UR_ASSERT(!(Command && !CommandBuffer->IsUpdatable),
+            UR_RESULT_ERROR_INVALID_OPERATION);
 
   // Lock automatically releases when this goes out of scope.
   std::scoped_lock<ur_shared_mutex, ur_shared_mutex, ur_shared_mutex> Lock(
@@ -770,7 +773,7 @@ ur_result_t urCommandBufferAppendKernelLaunchExp(
   // reference count on the kernel, using the kernel saved in CommandData.
   UR_CALL(ur::level_zero::urKernelRetain(Kernel));
 
-  if (Command && CommandBuffer->IsUpdatable) {
+  if (Command) {
     UR_CALL(createCommandHandle(CommandBuffer, Kernel, WorkDim, LocalWorkSize,
                                 *Command));
   }
@@ -1605,14 +1608,14 @@ ur_result_t updateKernelCommand(
 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->CommandBuffer->IsUpdatable,
+            UR_RESULT_ERROR_INVALID_OPERATION);
   UR_ASSERT(Command->Kernel, UR_RESULT_ERROR_INVALID_NULL_HANDLE);
 
   // Lock command, kernel and command buffer for update.
   std::scoped_lock<ur_shared_mutex, ur_shared_mutex, ur_shared_mutex> Guard(
       Command->Mutex, Command->CommandBuffer->Mutex, Command->Kernel->Mutex);
 
-  UR_ASSERT(Command->CommandBuffer->IsUpdatable,
-            UR_RESULT_ERROR_INVALID_OPERATION);
   UR_ASSERT(Command->CommandBuffer->IsFinalized,
             UR_RESULT_ERROR_INVALID_OPERATION);
 
@@ -1641,6 +1644,16 @@ urCommandBufferGetInfoExp(ur_exp_command_buffer_handle_t hCommandBuffer,
   switch (propName) {
   case UR_EXP_COMMAND_BUFFER_INFO_REFERENCE_COUNT:
     return ReturnValue(uint32_t{hCommandBuffer->RefCount.load()});
+  case UR_EXP_COMMAND_BUFFER_INFO_DESCRIPTOR: {
+    ur_exp_command_buffer_desc_t Descriptor{};
+    Descriptor.stype = UR_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_DESC;
+    Descriptor.pNext = nullptr;
+    Descriptor.isUpdatable = hCommandBuffer->IsUpdatable;
+    Descriptor.isInOrder = hCommandBuffer->IsInOrderCmdList;
+    Descriptor.enableProfiling = hCommandBuffer->IsProfilingEnabled;
+
+    return ReturnValue(Descriptor);
+  }
   default:
     assert(!"Command-buffer info request not implemented");
   }

diff --git a/source/adapters/mock/ur_mockddi.cpp b/source/adapters/mock/ur_mockddi.cpp
@@ -8368,8 +8368,9 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferAppendKernelLaunchExp(
                             ///< be ignored if command-buffer is in-order.
     ur_exp_command_buffer_sync_point_t *
         pSyncPoint, ///< [out][optional] Sync point associated with this command.
-    ur_exp_command_buffer_command_handle_t
-        *phCommand ///< [out][optional] Handle to this command.
+    ur_exp_command_buffer_command_handle_t *
+        phCommand ///< [out][optional] Handle to this command. Only available if the
+                  ///< command-buffer is updatable.
     ) try {
     ur_result_t result = UR_RESULT_SUCCESS;
 

diff --git a/source/adapters/opencl/command_buffer.cpp b/source/adapters/opencl/command_buffer.cpp
@@ -148,6 +148,10 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferAppendKernelLaunchExp(
     ur_exp_command_buffer_sync_point_t *pSyncPoint,
     ur_exp_command_buffer_command_handle_t *phCommandHandle) {
 
+  // Command handles can only be obtained from updatable command-buffers
+  UR_ASSERT(!(phCommandHandle && !hCommandBuffer->IsUpdatable),
+            UR_RESULT_ERROR_INVALID_OPERATION);
+
   cl_context CLContext = cl_adapter::cast<cl_context>(hCommandBuffer->hContext);
   cl_ext::clCommandNDRangeKernelKHR_fn clCommandNDRangeKernelKHR = nullptr;
   UR_RETURN_ON_FAILURE(
@@ -580,6 +584,16 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferGetInfoExp(
   switch (propName) {
   case UR_EXP_COMMAND_BUFFER_INFO_REFERENCE_COUNT:
     return ReturnValue(hCommandBuffer->getExternalReferenceCount());
+  case UR_EXP_COMMAND_BUFFER_INFO_DESCRIPTOR: {
+    ur_exp_command_buffer_desc_t Descriptor{};
+    Descriptor.stype = UR_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_DESC;
+    Descriptor.pNext = nullptr;
+    Descriptor.isUpdatable = hCommandBuffer->IsUpdatable;
+    Descriptor.isInOrder = false;
+    Descriptor.enableProfiling = false;
+
+    return ReturnValue(Descriptor);
+  }
   default:
     assert(!"Command-buffer info request not implemented");
   }

diff --git a/source/loader/layers/tracing/ur_trcddi.cpp b/source/loader/layers/tracing/ur_trcddi.cpp
@@ -7154,8 +7154,9 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferAppendKernelLaunchExp(
                             ///< be ignored if command-buffer is in-order.
     ur_exp_command_buffer_sync_point_t *
         pSyncPoint, ///< [out][optional] Sync point associated with this command.
-    ur_exp_command_buffer_command_handle_t
-        *phCommand ///< [out][optional] Handle to this command.
+    ur_exp_command_buffer_command_handle_t *
+        phCommand ///< [out][optional] Handle to this command. Only available if the
+                  ///< command-buffer is updatable.
 ) {
     auto pfnAppendKernelLaunchExp =
         getContext()->urDdiTable.CommandBufferExp.pfnAppendKernelLaunchExp;

diff --git a/source/loader/layers/validation/ur_valddi.cpp b/source/loader/layers/validation/ur_valddi.cpp
@@ -8077,8 +8077,9 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferAppendKernelLaunchExp(
                             ///< be ignored if command-buffer is in-order.
     ur_exp_command_buffer_sync_point_t *
         pSyncPoint, ///< [out][optional] Sync point associated with this command.
-    ur_exp_command_buffer_command_handle_t
-        *phCommand ///< [out][optional] Handle to this command.
+    ur_exp_command_buffer_command_handle_t *
+        phCommand ///< [out][optional] Handle to this command. Only available if the
+                  ///< command-buffer is updatable.
 ) {
     auto pfnAppendKernelLaunchExp =
         getContext()->urDdiTable.CommandBufferExp.pfnAppendKernelLaunchExp;
@@ -9005,7 +9006,7 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferGetInfoExp(
             return UR_RESULT_ERROR_INVALID_NULL_POINTER;
         }
 
-        if (UR_EXP_COMMAND_BUFFER_INFO_REFERENCE_COUNT < propName) {
+        if (UR_EXP_COMMAND_BUFFER_INFO_DESCRIPTOR < propName) {
             return UR_RESULT_ERROR_INVALID_ENUMERATION;
         }
 

diff --git a/source/loader/ur_ldrddi.cpp b/source/loader/ur_ldrddi.cpp
@@ -7124,8 +7124,9 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferAppendKernelLaunchExp(
                             ///< be ignored if command-buffer is in-order.
     ur_exp_command_buffer_sync_point_t *
         pSyncPoint, ///< [out][optional] Sync point associated with this command.
-    ur_exp_command_buffer_command_handle_t
-        *phCommand ///< [out][optional] Handle to this command.
+    ur_exp_command_buffer_command_handle_t *
+        phCommand ///< [out][optional] Handle to this command. Only available if the
+                  ///< command-buffer is updatable.
 ) {
     ur_result_t result = UR_RESULT_SUCCESS;
 

diff --git a/source/loader/ur_libapi.cpp b/source/loader/ur_libapi.cpp
@@ -7535,6 +7535,7 @@ ur_result_t UR_APICALL urCommandBufferFinalizeExp(
 ///         + `pSyncPointWaitList != NULL && numSyncPointsInWaitList == 0`
 ///     - ::UR_RESULT_ERROR_OUT_OF_HOST_MEMORY
 ///     - ::UR_RESULT_ERROR_OUT_OF_RESOURCES
+///     - ::UR_RESULT_ERROR_INVALID_OPERATION - "phCommand is not NULL and hCommandBuffer is not updatable."
 ur_result_t UR_APICALL urCommandBufferAppendKernelLaunchExp(
     ur_exp_command_buffer_handle_t
         hCommandBuffer,         ///< [in] Handle of the command-buffer object.
@@ -7564,8 +7565,9 @@ ur_result_t UR_APICALL urCommandBufferAppendKernelLaunchExp(
                             ///< be ignored if command-buffer is in-order.
     ur_exp_command_buffer_sync_point_t *
         pSyncPoint, ///< [out][optional] Sync point associated with this command.
-    ur_exp_command_buffer_command_handle_t
-        *phCommand ///< [out][optional] Handle to this command.
+    ur_exp_command_buffer_command_handle_t *
+        phCommand ///< [out][optional] Handle to this command. Only available if the
+                  ///< command-buffer is updatable.
     ) try {
     auto pfnAppendKernelLaunchExp =
         ur_lib::getContext()
@@ -8358,7 +8360,7 @@ ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp(
 ///     - ::UR_RESULT_ERROR_INVALID_NULL_HANDLE
 ///         + `NULL == hCommandBuffer`
 ///     - ::UR_RESULT_ERROR_INVALID_ENUMERATION
-///         + `::UR_EXP_COMMAND_BUFFER_INFO_REFERENCE_COUNT < propName`
+///         + `::UR_EXP_COMMAND_BUFFER_INFO_DESCRIPTOR < propName`
 ///     - ::UR_RESULT_ERROR_UNSUPPORTED_ENUMERATION
 ///         + If `propName` is not supported by the adapter.
 ///     - ::UR_RESULT_ERROR_INVALID_SIZE

diff --git a/source/ur_api.cpp b/source/ur_api.cpp
@@ -6391,6 +6391,7 @@ ur_result_t UR_APICALL urCommandBufferFinalizeExp(
 ///         + `pSyncPointWaitList != NULL && numSyncPointsInWaitList == 0`
 ///     - ::UR_RESULT_ERROR_OUT_OF_HOST_MEMORY
 ///     - ::UR_RESULT_ERROR_OUT_OF_RESOURCES
+///     - ::UR_RESULT_ERROR_INVALID_OPERATION - "phCommand is not NULL and hCommandBuffer is not updatable."
 ur_result_t UR_APICALL urCommandBufferAppendKernelLaunchExp(
     ur_exp_command_buffer_handle_t
         hCommandBuffer,         ///< [in] Handle of the command-buffer object.
@@ -6420,8 +6421,9 @@ ur_result_t UR_APICALL urCommandBufferAppendKernelLaunchExp(
                             ///< be ignored if command-buffer is in-order.
     ur_exp_command_buffer_sync_point_t *
         pSyncPoint, ///< [out][optional] Sync point associated with this command.
-    ur_exp_command_buffer_command_handle_t
-        *phCommand ///< [out][optional] Handle to this command.
+    ur_exp_command_buffer_command_handle_t *
+        phCommand ///< [out][optional] Handle to this command. Only available if the
+                  ///< command-buffer is updatable.
 ) {
     ur_result_t result = UR_RESULT_SUCCESS;
     return result;
@@ -7063,7 +7065,7 @@ ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp(
 ///     - ::UR_RESULT_ERROR_INVALID_NULL_HANDLE
 ///         + `NULL == hCommandBuffer`
 ///     - ::UR_RESULT_ERROR_INVALID_ENUMERATION
-///         + `::UR_EXP_COMMAND_BUFFER_INFO_REFERENCE_COUNT < propName`
+///         + `::UR_EXP_COMMAND_BUFFER_INFO_DESCRIPTOR < propName`
 ///     - ::UR_RESULT_ERROR_UNSUPPORTED_ENUMERATION
 ///         + If `propName` is not supported by the adapter.
 ///     - ::UR_RESULT_ERROR_INVALID_SIZE