From 9137082d6d10db27161856fb73f8164951943f50 Mon Sep 17 00:00:00 2001 From: Ewan Crawford Date: Thu, 4 Jan 2024 12:43:55 +0000 Subject: [PATCH] Cleanups to spec --- include/ur_api.h | 8 +++---- scripts/core/EXP-COMMAND-BUFFER.rst | 16 ++++++++------ scripts/core/exp-command-buffer.yml | 8 +++---- source/adapters/cuda/command_buffer.hpp | 22 ++++++++++++------- source/adapters/null/ur_nullddi.cpp | 2 +- source/loader/layers/tracing/ur_trcddi.cpp | 2 +- source/loader/layers/validation/ur_valddi.cpp | 2 +- source/loader/ur_ldrddi.cpp | 15 +------------ source/loader/ur_libapi.cpp | 2 +- source/ur_api.cpp | 2 +- .../testing/include/uur/fixtures.h | 2 +- 11 files changed, 38 insertions(+), 43 deletions(-) diff --git a/include/ur_api.h b/include/ur_api.h index 704087f6ed..bda08fa926 100644 --- a/include/ur_api.h +++ b/include/ur_api.h @@ -1537,9 +1537,9 @@ typedef enum ur_device_info_t { ///< version than older devices. UR_DEVICE_INFO_VIRTUAL_MEMORY_SUPPORT = 114, ///< [::ur_bool_t] return true if the device supports virtual memory. UR_DEVICE_INFO_ESIMD_SUPPORT = 115, ///< [::ur_bool_t] return true if the device supports ESIMD. - UR_DEVICE_INFO_COMMAND_BUFFER_SUPPORT_EXP = 0x1000, ///< [::ur_bool_t] returns true if the device supports the use of + UR_DEVICE_INFO_COMMAND_BUFFER_SUPPORT_EXP = 0x1000, ///< [::ur_bool_t] Returns true if the device supports the use of ///< command-buffers. - UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_SUPPORT_EXP = 0x1001, ///< [::ur_bool_t] returns true if the device supports updating the + UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_SUPPORT_EXP = 0x1001, ///< [::ur_bool_t] Returns true if the device supports updating the kernel ///< commands in a command-buffer. UR_DEVICE_INFO_BINDLESS_IMAGES_SUPPORT_EXP = 0x2000, ///< [::ur_bool_t] returns true if the device supports the creation of ///< bindless images @@ -7807,7 +7807,7 @@ typedef struct ur_exp_command_buffer_update_value_arg_desc_t { const void *pNext; ///< [in][optional] pointer to extension-specific structure uint32_t argIndex; ///< [in] Argument index. uint32_t argSize; ///< [in] Argument size. - const ur_kernel_arg_value_properties_t *pProperties; ///< [in][optinal] Pointer to memory object properties. + const ur_kernel_arg_value_properties_t *pProperties; ///< [in][optinal] Pointer to value properties. const void *pArgValue; ///< [in][optional] Argument value representing kernel arg type. } ur_exp_command_buffer_update_value_arg_desc_t; @@ -8461,7 +8461,7 @@ urCommandBufferEnqueueExp( UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp( ur_exp_command_buffer_command_handle_t hCommand, ///< [in] Handle of the command-buffer kernel command to update. - const ur_exp_command_buffer_update_kernel_launch_desc_t *pUpdateKernelLaunch ///< [in] Handle of the command-buffer kernel command to update. + const ur_exp_command_buffer_update_kernel_launch_desc_t *pUpdateKernelLaunch ///< [in] Struct defining how the kernel command is to be updated. ); #if !defined(__GNUC__) diff --git a/scripts/core/EXP-COMMAND-BUFFER.rst b/scripts/core/EXP-COMMAND-BUFFER.rst index b2cc4309d3..afe081f7e2 100644 --- a/scripts/core/EXP-COMMAND-BUFFER.rst +++ b/scripts/core/EXP-COMMAND-BUFFER.rst @@ -162,13 +162,13 @@ Updating Command-Buffers Commands An adapter implementing the command-buffer experimental feature can optionally support updating the configuration of kernel commands recorded to a -command-buffer. Support of this is reported by returning true in the +command-buffer. Support for this is reported by returning true in the ${X}_DEVICE_INFO_COMMAND_BUFFER_UPDATE_SUPPORT_EXP query. Updating kernel commands is done by passing the new kernel configuration to ${x}CommandBufferUpdateKernelLaunchExp along with the command handle of the kernel command to update. Configurations that can be changed are the -kernels ND-Range and parameters. +parameters to the kernel and the execution ND-Range. .. parsed-literal:: @@ -176,18 +176,18 @@ kernels ND-Range and parameters. ${x}_exp_command_buffer_desc_t desc { ${X}_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_DESC, nullptr, - true + true // isUpdatable }; ${x}_exp_command_buffer_handle_t hCommandBuffer; - ${x}CommandBufferCreateExp(hContext, hDevice, &desc, &handle); + ${x}CommandBufferCreateExp(hContext, hDevice, &desc, &hCommandBuffer); // Append a kernel command which has two buffer parameters, an input // and an output. - ${x}_exp_command_buffer_command_handle_t handle; + ${x}_exp_command_buffer_command_handle_t hCommand; ${x}CommandBufferAppendKernelLaunchExp(hCommandBuffer, hKernel, workDim, pGlobalWorkOffset, pGlobalWorkSize, pLocalWorkSize, 0, nullptr, - nullptr, &handle); + nullptr, &hCommand); // Close the command-buffer before updating ${x}CommandBufferFinalizeExp(hCommandBuffer); @@ -230,7 +230,7 @@ kernels ND-Range and parameters. }; // Perform the update - ${x}CommandBufferUpdateKernelLaunchExp(handle, &update); + ${x}CommandBufferUpdateKernelLaunchExp(hCommand, &update); API @@ -255,6 +255,7 @@ Enums * ${X}_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_UPDATE_KERNEL_LAUNCH_DESC * ${X}_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_UPDATE_MEMOBJ_ARG_DESC * ${X}_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_UPDATE_POINTER_ARG_DESC + * ${X}_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_UPDATE_VALUE_ARG_DESC * ${X}_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_UPDATE_EXEC_INFO_DESC * ${x}_command_t * ${X}_COMMAND_COMMAND_BUFFER_ENQUEUE_EXP @@ -284,6 +285,7 @@ Types * ${x}_exp_command_buffer_update_kernel_launch_desc_t * ${x}_exp_command_buffer_update_memobj_arg_desc_t * ${x}_exp_command_buffer_update_pointer_arg_desc_t +* ${x}_exp_command_buffer_update_value_arg_desc_t * ${x}_exp_command_buffer_update_exec_info_desc_t * ${x}_exp_command_buffer_sync_point_t * ${x}_exp_command_buffer_handle_t diff --git a/scripts/core/exp-command-buffer.yml b/scripts/core/exp-command-buffer.yml index f0a023eac0..568b4896e0 100644 --- a/scripts/core/exp-command-buffer.yml +++ b/scripts/core/exp-command-buffer.yml @@ -20,10 +20,10 @@ name: $x_device_info_t etors: - name: COMMAND_BUFFER_SUPPORT_EXP value: "0x1000" - desc: "[$x_bool_t] returns true if the device supports the use of command-buffers." + desc: "[$x_bool_t] Returns true if the device supports the use of command-buffers." - name: COMMAND_BUFFER_UPDATE_SUPPORT_EXP value: "0x1001" - desc: "[$x_bool_t] returns true if the device supports updating the commands in a command-buffer." + desc: "[$x_bool_t] Returns true if the device supports updating the kernel commands in a command-buffer." --- #-------------------------------------------------------------------------- type: enum @@ -135,7 +135,7 @@ members: desc: "[in] Argument size." - type: "const ur_kernel_arg_value_properties_t *" name: pProperties - desc: "[in][optinal] Pointer to memory object properties." + desc: "[in][optinal] Pointer to value properties." - type: "const void *" name: pArgValue desc: "[in][optional] Argument value representing kernel arg type." @@ -872,7 +872,7 @@ params: desc: "[in] Handle of the command-buffer kernel command to update." - type: "const $x_exp_command_buffer_update_kernel_launch_desc_t*" name: pUpdateKernelLaunch - desc: "[in] Handle of the command-buffer kernel command to update." + desc: "[in] Struct defining how the kernel command is to be updated." returns: - $X_RESULT_ERROR_UNSUPPORTED_FEATURE: diff --git a/source/adapters/cuda/command_buffer.hpp b/source/adapters/cuda/command_buffer.hpp index c31c053f2a..74825870ba 100644 --- a/source/adapters/cuda/command_buffer.hpp +++ b/source/adapters/cuda/command_buffer.hpp @@ -175,9 +175,11 @@ static inline const char *getUrResultString(ur_result_t Result) { fprintf(stderr, "UR <--- %s(%s)\n", #Call, getUrResultString(Result)); \ } -// Handle type specific to kernel command. Will need to -// be refactored when handles can be returned from other -// command types. +// Handle to a kernel command. +// +// Struct that stores all the information related to a kernel command in a +// command-buffer, such that the command can be recreated. When handles can +// be returned from other command types this struct will need refactored. struct ur_exp_command_buffer_command_handle_t_ { ur_exp_command_buffer_command_handle_t_( ur_exp_command_buffer_handle_t CommandBuffer, ur_kernel_handle_t Kernel, @@ -218,11 +220,11 @@ struct ur_exp_command_buffer_command_handle_t_ { } void SetLocalSize(const size_t *pLocalWorkSize) { - const size_t copy_size = sizeof(size_t) * WorkDim; - std::memcpy(LocalWorkSize, pLocalWorkSize, copy_size); + const size_t CopySize = sizeof(size_t) * WorkDim; + std::memcpy(LocalWorkSize, pLocalWorkSize, CopySize); if (WorkDim < 3) { - const size_t zero_size = sizeof(size_t) * (3 - WorkDim); - std::memset(LocalWorkSize + WorkDim, 0, zero_size); + const size_t ZeroSize = sizeof(size_t) * (3 - WorkDim); + std::memset(LocalWorkSize + WorkDim, 0, ZeroSize); } } @@ -267,7 +269,11 @@ struct ur_exp_command_buffer_handle_t_ { // Creates a UR command handle // @param[in] Kernel UR kernel associated with this command. // @param[in] Node CUDA Graph node associated with this command. - // @param[in] Params Kernel configuration associated with this node. + // @param[in] Params CUDA Kernel configuration associated with this node. + // @param[in] WorkDim Dimensions of the kernel execution. + // @param[in] GlobalWorkOffset Work item offset of the kernel execution. + // @param[in] GlobalWorkSize Global work size of the kernel execution. + // @param[in] LocalWorkSize Local work size of the kernel execution. // @return Shared pointer to the created handle. std::shared_ptr AddCommandHandle(ur_kernel_handle_t Kernel, std::shared_ptr Node, diff --git a/source/adapters/null/ur_nullddi.cpp b/source/adapters/null/ur_nullddi.cpp index 79bf47fb18..60988eb22c 100644 --- a/source/adapters/null/ur_nullddi.cpp +++ b/source/adapters/null/ur_nullddi.cpp @@ -5029,7 +5029,7 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp( ur_exp_command_buffer_command_handle_t hCommand, ///< [in] Handle of the command-buffer kernel command to update. const ur_exp_command_buffer_update_kernel_launch_desc_t * - pUpdateKernelLaunch ///< [in] Handle of the command-buffer kernel command to update. + pUpdateKernelLaunch ///< [in] Struct defining how the kernel command is to be updated. ) try { ur_result_t result = UR_RESULT_SUCCESS; diff --git a/source/loader/layers/tracing/ur_trcddi.cpp b/source/loader/layers/tracing/ur_trcddi.cpp index 114fac878c..b3e7f10f37 100644 --- a/source/loader/layers/tracing/ur_trcddi.cpp +++ b/source/loader/layers/tracing/ur_trcddi.cpp @@ -5815,7 +5815,7 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp( ur_exp_command_buffer_command_handle_t hCommand, ///< [in] Handle of the command-buffer kernel command to update. const ur_exp_command_buffer_update_kernel_launch_desc_t * - pUpdateKernelLaunch ///< [in] Handle of the command-buffer kernel command to update. + pUpdateKernelLaunch ///< [in] Struct defining how the kernel command is to be updated. ) { auto pfnUpdateKernelLaunchExp = context.urDdiTable.CommandBufferExp.pfnUpdateKernelLaunchExp; diff --git a/source/loader/layers/validation/ur_valddi.cpp b/source/loader/layers/validation/ur_valddi.cpp index 7a2355167f..9b137051bc 100644 --- a/source/loader/layers/validation/ur_valddi.cpp +++ b/source/loader/layers/validation/ur_valddi.cpp @@ -7623,7 +7623,7 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp( ur_exp_command_buffer_command_handle_t hCommand, ///< [in] Handle of the command-buffer kernel command to update. const ur_exp_command_buffer_update_kernel_launch_desc_t * - pUpdateKernelLaunch ///< [in] Handle of the command-buffer kernel command to update. + pUpdateKernelLaunch ///< [in] Struct defining how the kernel command is to be updated. ) { auto pfnUpdateKernelLaunchExp = context.urDdiTable.CommandBufferExp.pfnUpdateKernelLaunchExp; diff --git a/source/loader/ur_ldrddi.cpp b/source/loader/ur_ldrddi.cpp index cb10d094b1..dcd4fa7a1d 100644 --- a/source/loader/ur_ldrddi.cpp +++ b/source/loader/ur_ldrddi.cpp @@ -6866,7 +6866,7 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp( ur_exp_command_buffer_command_handle_t hCommand, ///< [in] Handle of the command-buffer kernel command to update. const ur_exp_command_buffer_update_kernel_launch_desc_t * - pUpdateKernelLaunch ///< [in] Handle of the command-buffer kernel command to update. + pUpdateKernelLaunch ///< [in] Struct defining how the kernel command is to be updated. ) { ur_result_t result = UR_RESULT_SUCCESS; @@ -6885,19 +6885,6 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp( reinterpret_cast(hCommand) ->handle; - uint32_t NumMemobjArgs = pUpdateKernelLaunch->numMemobjArgs; - if (NumMemobjArgs) { - for (uint32_t i = 0; i < NumMemobjArgs; i++) { - auto &MemobjArgDesc = pUpdateKernelLaunch->pArgMemobjList[i]; - ur_mem_handle_t &ArgValue = - const_cast(MemobjArgDesc.hArgValue); - ArgValue = - (ArgValue) - ? reinterpret_cast(ArgValue)->handle - : nullptr; - } - } - // forward to device-platform result = pfnUpdateKernelLaunchExp(hCommand, pUpdateKernelLaunch); diff --git a/source/loader/ur_libapi.cpp b/source/loader/ur_libapi.cpp index 32ae64b35a..47c0bc0375 100644 --- a/source/loader/ur_libapi.cpp +++ b/source/loader/ur_libapi.cpp @@ -7864,7 +7864,7 @@ ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp( ur_exp_command_buffer_command_handle_t hCommand, ///< [in] Handle of the command-buffer kernel command to update. const ur_exp_command_buffer_update_kernel_launch_desc_t * - pUpdateKernelLaunch ///< [in] Handle of the command-buffer kernel command to update. + pUpdateKernelLaunch ///< [in] Struct defining how the kernel command is to be updated. ) try { auto pfnUpdateKernelLaunchExp = ur_lib::context->urDdiTable.CommandBufferExp.pfnUpdateKernelLaunchExp; diff --git a/source/ur_api.cpp b/source/ur_api.cpp index 01bc8bc9aa..f33d107470 100644 --- a/source/ur_api.cpp +++ b/source/ur_api.cpp @@ -6649,7 +6649,7 @@ ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp( ur_exp_command_buffer_command_handle_t hCommand, ///< [in] Handle of the command-buffer kernel command to update. const ur_exp_command_buffer_update_kernel_launch_desc_t * - pUpdateKernelLaunch ///< [in] Handle of the command-buffer kernel command to update. + pUpdateKernelLaunch ///< [in] Struct defining how the kernel command is to be updated. ) { ur_result_t result = UR_RESULT_SUCCESS; return result; diff --git a/test/conformance/testing/include/uur/fixtures.h b/test/conformance/testing/include/uur/fixtures.h index 0da9bdc22f..fed57fcb38 100644 --- a/test/conformance/testing/include/uur/fixtures.h +++ b/test/conformance/testing/include/uur/fixtures.h @@ -1255,7 +1255,7 @@ struct urBaseKernelExecutionTest : urBaseKernelTest { }; struct urKernelExecutionTest : urBaseKernelExecutionTest { - void SetUp() override { + void SetUp() { UUR_RETURN_ON_FATAL_FAILURE(urBaseKernelExecutionTest::SetUp()); Build(); }