Error if updatable command-buffer created without device support

oneapi-src · Feb 22, 2024 · be61020 · be61020
1 parent be715c7
commit be61020
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 8 deletions.
diff --git a/include/ur_api.h b/include/ur_api.h
@@ -7929,6 +7929,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_OUT_OF_HOST_MEMORY
 ///     - ::UR_RESULT_ERROR_OUT_OF_RESOURCES
 UR_APIEXPORT ur_result_t UR_APICALL

diff --git a/scripts/core/exp-command-buffer.yml b/scripts/core/exp-command-buffer.yml
@@ -261,6 +261,8 @@ 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_OUT_OF_HOST_MEMORY
     - $X_RESULT_ERROR_OUT_OF_RESOURCES
 --- #--------------------------------------------------------------------------

diff --git a/source/adapters/opencl/command_buffer.cpp b/source/adapters/opencl/command_buffer.cpp
@@ -48,6 +48,7 @@ ur_exp_command_buffer_handle_t_::~ur_exp_command_buffer_handle_t_() {
           CLContext, cl_ext::ExtFuncPtrCache->clReleaseCommandBufferKHRCache,
           cl_ext::ReleaseCommandBufferName, &clReleaseCommandBufferKHR);
   assert(Res == CL_SUCCESS);
+  (void)Res;
 
   clReleaseCommandBufferKHR(CLCommandBuffer);
 }
@@ -73,23 +74,25 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferCreateExp(
   const bool IsUpdatable =
       pCommandBufferDesc ? pCommandBufferDesc->isUpdatable : false;
 
-  bool SupportsUpdate = false;
+  bool DeviceSupportsUpdate = false;
   cl_device_id CLDevice = cl_adapter::cast<cl_device_id>(hDevice);
-  CL_RETURN_ON_FAILURE(
-      deviceSupportsURCommandBufferKernelUpdate(CLDevice, SupportsUpdate));
+  CL_RETURN_ON_FAILURE(deviceSupportsURCommandBufferKernelUpdate(
+      CLDevice, DeviceSupportsUpdate));
 
-  const bool Updatable = IsUpdatable && SupportsUpdate;
+  if (IsUpdatable && !DeviceSupportsUpdate) {
+    return UR_RESULT_ERROR_INVALID_OPERATION;
+  }
 
   cl_command_buffer_properties_khr Properties[3] = {
       CL_COMMAND_BUFFER_FLAGS_KHR,
-      Updatable ? CL_COMMAND_BUFFER_MUTABLE_KHR : 0u, 0};
+      IsUpdatable ? CL_COMMAND_BUFFER_MUTABLE_KHR : 0u, 0};
   auto CLCommandBuffer = clCreateCommandBufferKHR(
       1, cl_adapter::cast<cl_command_queue *>(&Queue), Properties, &Res);
   CL_RETURN_ON_FAILURE_AND_SET_NULL(Res, phCommandBuffer);
 
   try {
     auto URCommandBuffer = std::make_unique<ur_exp_command_buffer_handle_t_>(
-        Queue, hContext, CLCommandBuffer, Updatable);
+        Queue, hContext, CLCommandBuffer, IsUpdatable);
     *phCommandBuffer = URCommandBuffer.release();
   } catch (...) {
     return UR_RESULT_ERROR_OUT_OF_RESOURCES;
@@ -499,7 +502,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp(
         ArgPointerList[i];
     cl_mutable_dispatch_arg_khr &USMArg = CLUSMArgs[i];
     USMArg.arg_index = URPointerArg.argIndex;
-    USMArg.arg_value = *(void **)URPointerArg.pNewPointerArg;
+    USMArg.arg_value = *(void *const *)URPointerArg.pNewPointerArg;
   }
 
   // Find the memory object and scalar arguments to the kernel.

diff --git a/source/loader/ur_libapi.cpp b/source/loader/ur_libapi.cpp
@@ -7030,6 +7030,8 @@ ur_result_t UR_APICALL urBindlessImagesSignalExternalSemaphoreExp(
 ///         + `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_OUT_OF_HOST_MEMORY
 ///     - ::UR_RESULT_ERROR_OUT_OF_RESOURCES
 ur_result_t UR_APICALL urCommandBufferCreateExp(

diff --git a/source/ur_api.cpp b/source/ur_api.cpp
@@ -5966,6 +5966,8 @@ ur_result_t UR_APICALL urBindlessImagesSignalExternalSemaphoreExp(
 ///         + `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_OUT_OF_HOST_MEMORY
 ///     - ::UR_RESULT_ERROR_OUT_OF_RESOURCES
 ur_result_t UR_APICALL urCommandBufferCreateExp(

diff --git a/test/conformance/exp_command_buffer/invalid_update.cpp b/test/conformance/exp_command_buffer/invalid_update.cpp
@@ -43,7 +43,10 @@ struct InvalidUpdateTest
     void TearDown() override {
         // Workaround an issue with the OpenCL adapter implementing urUsmFree
         // using a blocking free where hangs
-        EXPECT_SUCCESS(urCommandBufferFinalizeExp(updatable_cmd_buf_handle));
+        if (updatable_cmd_buf_handle) {
+            EXPECT_SUCCESS(
+                urCommandBufferFinalizeExp(updatable_cmd_buf_handle));
+        }
 
         if (shared_ptr) {
             EXPECT_SUCCESS(urUSMFree(context, shared_ptr));