From c9be1e28d2e2a132e8a40d707175d6625d50d8f9 Mon Sep 17 00:00:00 2001 From: Artur Gainullin Date: Mon, 4 Mar 2024 07:00:43 -0800 Subject: [PATCH] Address feedback * Remove unnecesary env variable setting * Check support of the feature on device * Check sub-feature support before usage at urCommandBufferUpdateKernelLaunchExp * Remove redundant code --- source/adapters/level_zero/command_buffer.cpp | 22 ++++++++++++++- source/adapters/level_zero/common.cpp | 5 ++++ source/adapters/level_zero/device.cpp | 27 +++++++++++++++++-- source/adapters/level_zero/device.hpp | 2 ++ .../conformance/exp_command_buffer/fixtures.h | 2 -- 5 files changed, 53 insertions(+), 5 deletions(-) diff --git a/source/adapters/level_zero/command_buffer.cpp b/source/adapters/level_zero/command_buffer.cpp index e6e836590e..36cf76d111 100644 --- a/source/adapters/level_zero/command_buffer.cpp +++ b/source/adapters/level_zero/command_buffer.cpp @@ -25,7 +25,6 @@ ur_exp_command_buffer_handle_t_::ur_exp_command_buffer_handle_t_( ZeCommandListDesc(ZeDesc), ZeFencesList(), QueueProperties(), SyncPoints(), NextSyncPoint(0), IsUpdatable(Desc ? Desc->isUpdatable : false) { - (void)Desc; urContextRetain(Context); urDeviceRetain(Device); } @@ -1045,6 +1044,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp( auto CommandBuffer = Command->CommandBuffer; uint32_t Dim = CommandDesc->newWorkDim; const void *NextDesc = nullptr; + auto SupportedFeatures = + Command->CommandBuffer->Device->ZeDeviceMutableCmdListsProperties + ->mutableCommandFlags; // We need the created descriptors to live till the point when // zexCommandListUpdateMutableCommandsExp is called at the end of the @@ -1060,6 +1062,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp( // 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 (!CommandBuffer->Context->getPlatform() ->ZeDriverGlobalOffsetExtensionFound) { @@ -1079,6 +1084,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp( // Check if new group size is provided. size_t *NewLocalWorkSize = CommandDesc->pNewLocalWorkSize; + UR_ASSERT(!NewLocalWorkSize || + (SupportedFeatures & ZE_MUTABLE_COMMAND_EXP_FLAG_GROUP_SIZE), + UR_RESULT_ERROR_UNSUPPORTED_FEATURE); if (NewLocalWorkSize && Dim > 0) { auto MutableGroupSizeDesc = std::make_unique>(); @@ -1093,6 +1101,12 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp( // Check if new global size is provided and we need to update group count. size_t *NewGlobalWorkSize = CommandDesc->pNewGlobalWorkSize; + UR_ASSERT(!NewGlobalWorkSize || + (SupportedFeatures & ZE_MUTABLE_COMMAND_EXP_FLAG_GROUP_COUNT), + UR_RESULT_ERROR_UNSUPPORTED_FEATURE); + UR_ASSERT(!(NewGlobalWorkSize && !NewLocalWorkSize) || + (SupportedFeatures & ZE_MUTABLE_COMMAND_EXP_FLAG_GROUP_SIZE), + UR_RESULT_ERROR_UNSUPPORTED_FEATURE); if (NewGlobalWorkSize && Dim > 0) { ze_group_count_t ZeThreadGroupDimensions{1, 1, 1}; uint32_t WG[3]; @@ -1124,6 +1138,12 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp( } } + UR_ASSERT( + (!CommandDesc->numNewMemObjArgs && !CommandDesc->numNewPointerArgs && + !CommandDesc->numNewValueArgs) || + (SupportedFeatures & ZE_MUTABLE_COMMAND_EXP_FLAG_KERNEL_ARGUMENTS), + UR_RESULT_ERROR_UNSUPPORTED_FEATURE); + // Check if new memory object arguments are provided. for (uint32_t NewMemObjArgNum = CommandDesc->numNewMemObjArgs; NewMemObjArgNum-- > 0;) { diff --git a/source/adapters/level_zero/common.cpp b/source/adapters/level_zero/common.cpp index 0377993077..a927c8b444 100644 --- a/source/adapters/level_zero/common.cpp +++ b/source/adapters/level_zero/common.cpp @@ -174,6 +174,11 @@ template <> ze_structure_type_t getZeStructureType() { return ZE_STRUCTURE_TYPE_COMMAND_LIST_DESC; } template <> +ze_structure_type_t +getZeStructureType() { + return ZE_STRUCTURE_TYPE_MUTABLE_COMMAND_LIST_EXP_PROPERTIES; +} +template <> ze_structure_type_t getZeStructureType() { return ZE_STRUCTURE_TYPE_MUTABLE_COMMAND_LIST_EXP_DESC; } diff --git a/source/adapters/level_zero/device.cpp b/source/adapters/level_zero/device.cpp index 9c08f38dc5..5d0fbdbc11 100644 --- a/source/adapters/level_zero/device.cpp +++ b/source/adapters/level_zero/device.cpp @@ -917,8 +917,22 @@ UR_APIEXPORT ur_result_t UR_APICALL urDeviceGetInfo( } case UR_DEVICE_INFO_COMMAND_BUFFER_SUPPORT_EXP: return ReturnValue(true); - case UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_SUPPORT_EXP: - return ReturnValue(Device->Platform->ZeMutableCmdListExt.Supported); + case UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_SUPPORT_EXP: { + // TODO: Level Zero API allows to check support for all sub-features: + // ZE_MUTABLE_COMMAND_EXP_FLAG_KERNEL_ARGUMENTS, + // ZE_MUTABLE_COMMAND_EXP_FLAG_GROUP_COUNT, + // ZE_MUTABLE_COMMAND_EXP_FLAG_GROUP_SIZE, + // ZE_MUTABLE_COMMAND_EXP_FLAG_GLOBAL_OFFSET, + // ZE_MUTABLE_COMMAND_EXP_FLAG_SIGNAL_EVENT, + // ZE_MUTABLE_COMMAND_EXP_FLAG_WAIT_EVENTS + // but UR has only one property to check the mutable command lists feature + // support. For now return true if kernel arguments can be updated. + auto KernelArgUpdateSupport = + Device->ZeDeviceMutableCmdListsProperties->mutableCommandFlags & + ZE_MUTABLE_COMMAND_EXP_FLAG_KERNEL_ARGUMENTS; + return ReturnValue(KernelArgUpdateSupport && + Device->Platform->ZeMutableCmdListExt.Supported); + } case UR_DEVICE_INFO_BINDLESS_IMAGES_SUPPORT_EXP: return ReturnValue(true); case UR_DEVICE_INFO_BINDLESS_IMAGES_SHARED_USM_SUPPORT_EXP: @@ -1142,6 +1156,15 @@ ur_result_t ur_device_handle_t_::initialize(int SubSubDeviceOrdinal, (ZeDevice, &Count, &Properties)); }; + ZeDeviceMutableCmdListsProperties.Compute = + [ZeDevice]( + ZeStruct &Properties) { + ze_device_properties_t P; + P.stype = ZE_STRUCTURE_TYPE_DEVICE_PROPERTIES; + P.pNext = &Properties; + ZE_CALL_NOCHECK(zeDeviceGetProperties, (ZeDevice, &P)); + }; + ImmCommandListUsed = this->useImmediateCommandLists(); uint32_t numQueueGroups = 0; diff --git a/source/adapters/level_zero/device.hpp b/source/adapters/level_zero/device.hpp index a57a97d38d..484890670b 100644 --- a/source/adapters/level_zero/device.hpp +++ b/source/adapters/level_zero/device.hpp @@ -195,4 +195,6 @@ struct ur_device_handle_t_ : _ur_object { ZeCache> ZeDeviceCacheProperties; ZeCache> ZeDeviceIpVersionExt; ZeCache ZeGlobalMemSize; + ZeCache> + ZeDeviceMutableCmdListsProperties; }; diff --git a/test/conformance/exp_command_buffer/fixtures.h b/test/conformance/exp_command_buffer/fixtures.h index 950fcaecd3..2f9656c0f9 100644 --- a/test/conformance/exp_command_buffer/fixtures.h +++ b/test/conformance/exp_command_buffer/fixtures.h @@ -113,9 +113,7 @@ struct urUpdatableCommandBufferExpExecutionTest } // Currently level zero driver doesn't support updating execution range. - // Also disable immediate command lists because there is synchronization issue causing test failures. if (backend == UR_PLATFORM_BACKEND_LEVEL_ZERO) { - setenv("UR_L0_USE_IMMEDIATE_COMMANDLISTS", "0", 0); updatable_execution_range_support = false; }