Skip to content

Commit c9be1e2

Browse files
committed
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
1 parent 72c43a8 commit c9be1e2

File tree

5 files changed

+53
-5
lines changed

5 files changed

+53
-5
lines changed

source/adapters/level_zero/command_buffer.cpp

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ ur_exp_command_buffer_handle_t_::ur_exp_command_buffer_handle_t_(
2525
ZeCommandListDesc(ZeDesc), ZeFencesList(), QueueProperties(),
2626
SyncPoints(), NextSyncPoint(0),
2727
IsUpdatable(Desc ? Desc->isUpdatable : false) {
28-
(void)Desc;
2928
urContextRetain(Context);
3029
urDeviceRetain(Device);
3130
}
@@ -1045,6 +1044,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp(
10451044
auto CommandBuffer = Command->CommandBuffer;
10461045
uint32_t Dim = CommandDesc->newWorkDim;
10471046
const void *NextDesc = nullptr;
1047+
auto SupportedFeatures =
1048+
Command->CommandBuffer->Device->ZeDeviceMutableCmdListsProperties
1049+
->mutableCommandFlags;
10481050

10491051
// We need the created descriptors to live till the point when
10501052
// zexCommandListUpdateMutableCommandsExp is called at the end of the
@@ -1060,6 +1062,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp(
10601062

10611063
// Check if new global offset is provided.
10621064
size_t *NewGlobalWorkOffset = CommandDesc->pNewGlobalWorkOffset;
1065+
UR_ASSERT(!NewGlobalWorkOffset ||
1066+
(SupportedFeatures & ZE_MUTABLE_COMMAND_EXP_FLAG_GLOBAL_OFFSET),
1067+
UR_RESULT_ERROR_UNSUPPORTED_FEATURE);
10631068
if (NewGlobalWorkOffset && Dim > 0) {
10641069
if (!CommandBuffer->Context->getPlatform()
10651070
->ZeDriverGlobalOffsetExtensionFound) {
@@ -1079,6 +1084,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp(
10791084

10801085
// Check if new group size is provided.
10811086
size_t *NewLocalWorkSize = CommandDesc->pNewLocalWorkSize;
1087+
UR_ASSERT(!NewLocalWorkSize ||
1088+
(SupportedFeatures & ZE_MUTABLE_COMMAND_EXP_FLAG_GROUP_SIZE),
1089+
UR_RESULT_ERROR_UNSUPPORTED_FEATURE);
10821090
if (NewLocalWorkSize && Dim > 0) {
10831091
auto MutableGroupSizeDesc =
10841092
std::make_unique<ZeStruct<ze_mutable_group_size_exp_desc_t>>();
@@ -1093,6 +1101,12 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp(
10931101

10941102
// Check if new global size is provided and we need to update group count.
10951103
size_t *NewGlobalWorkSize = CommandDesc->pNewGlobalWorkSize;
1104+
UR_ASSERT(!NewGlobalWorkSize ||
1105+
(SupportedFeatures & ZE_MUTABLE_COMMAND_EXP_FLAG_GROUP_COUNT),
1106+
UR_RESULT_ERROR_UNSUPPORTED_FEATURE);
1107+
UR_ASSERT(!(NewGlobalWorkSize && !NewLocalWorkSize) ||
1108+
(SupportedFeatures & ZE_MUTABLE_COMMAND_EXP_FLAG_GROUP_SIZE),
1109+
UR_RESULT_ERROR_UNSUPPORTED_FEATURE);
10961110
if (NewGlobalWorkSize && Dim > 0) {
10971111
ze_group_count_t ZeThreadGroupDimensions{1, 1, 1};
10981112
uint32_t WG[3];
@@ -1124,6 +1138,12 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp(
11241138
}
11251139
}
11261140

1141+
UR_ASSERT(
1142+
(!CommandDesc->numNewMemObjArgs && !CommandDesc->numNewPointerArgs &&
1143+
!CommandDesc->numNewValueArgs) ||
1144+
(SupportedFeatures & ZE_MUTABLE_COMMAND_EXP_FLAG_KERNEL_ARGUMENTS),
1145+
UR_RESULT_ERROR_UNSUPPORTED_FEATURE);
1146+
11271147
// Check if new memory object arguments are provided.
11281148
for (uint32_t NewMemObjArgNum = CommandDesc->numNewMemObjArgs;
11291149
NewMemObjArgNum-- > 0;) {

source/adapters/level_zero/common.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,11 @@ template <> ze_structure_type_t getZeStructureType<ze_command_list_desc_t>() {
174174
return ZE_STRUCTURE_TYPE_COMMAND_LIST_DESC;
175175
}
176176
template <>
177+
ze_structure_type_t
178+
getZeStructureType<ze_mutable_command_list_exp_properties_t>() {
179+
return ZE_STRUCTURE_TYPE_MUTABLE_COMMAND_LIST_EXP_PROPERTIES;
180+
}
181+
template <>
177182
ze_structure_type_t getZeStructureType<ze_mutable_command_list_exp_desc_t>() {
178183
return ZE_STRUCTURE_TYPE_MUTABLE_COMMAND_LIST_EXP_DESC;
179184
}

source/adapters/level_zero/device.cpp

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -917,8 +917,22 @@ UR_APIEXPORT ur_result_t UR_APICALL urDeviceGetInfo(
917917
}
918918
case UR_DEVICE_INFO_COMMAND_BUFFER_SUPPORT_EXP:
919919
return ReturnValue(true);
920-
case UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_SUPPORT_EXP:
921-
return ReturnValue(Device->Platform->ZeMutableCmdListExt.Supported);
920+
case UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_SUPPORT_EXP: {
921+
// TODO: Level Zero API allows to check support for all sub-features:
922+
// ZE_MUTABLE_COMMAND_EXP_FLAG_KERNEL_ARGUMENTS,
923+
// ZE_MUTABLE_COMMAND_EXP_FLAG_GROUP_COUNT,
924+
// ZE_MUTABLE_COMMAND_EXP_FLAG_GROUP_SIZE,
925+
// ZE_MUTABLE_COMMAND_EXP_FLAG_GLOBAL_OFFSET,
926+
// ZE_MUTABLE_COMMAND_EXP_FLAG_SIGNAL_EVENT,
927+
// ZE_MUTABLE_COMMAND_EXP_FLAG_WAIT_EVENTS
928+
// but UR has only one property to check the mutable command lists feature
929+
// support. For now return true if kernel arguments can be updated.
930+
auto KernelArgUpdateSupport =
931+
Device->ZeDeviceMutableCmdListsProperties->mutableCommandFlags &
932+
ZE_MUTABLE_COMMAND_EXP_FLAG_KERNEL_ARGUMENTS;
933+
return ReturnValue(KernelArgUpdateSupport &&
934+
Device->Platform->ZeMutableCmdListExt.Supported);
935+
}
922936
case UR_DEVICE_INFO_BINDLESS_IMAGES_SUPPORT_EXP:
923937
return ReturnValue(true);
924938
case UR_DEVICE_INFO_BINDLESS_IMAGES_SHARED_USM_SUPPORT_EXP:
@@ -1142,6 +1156,15 @@ ur_result_t ur_device_handle_t_::initialize(int SubSubDeviceOrdinal,
11421156
(ZeDevice, &Count, &Properties));
11431157
};
11441158

1159+
ZeDeviceMutableCmdListsProperties.Compute =
1160+
[ZeDevice](
1161+
ZeStruct<ze_mutable_command_list_exp_properties_t> &Properties) {
1162+
ze_device_properties_t P;
1163+
P.stype = ZE_STRUCTURE_TYPE_DEVICE_PROPERTIES;
1164+
P.pNext = &Properties;
1165+
ZE_CALL_NOCHECK(zeDeviceGetProperties, (ZeDevice, &P));
1166+
};
1167+
11451168
ImmCommandListUsed = this->useImmediateCommandLists();
11461169

11471170
uint32_t numQueueGroups = 0;

source/adapters/level_zero/device.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,4 +195,6 @@ struct ur_device_handle_t_ : _ur_object {
195195
ZeCache<ZeStruct<ze_device_cache_properties_t>> ZeDeviceCacheProperties;
196196
ZeCache<ZeStruct<ze_device_ip_version_ext_t>> ZeDeviceIpVersionExt;
197197
ZeCache<struct ze_global_memsize> ZeGlobalMemSize;
198+
ZeCache<ZeStruct<ze_mutable_command_list_exp_properties_t>>
199+
ZeDeviceMutableCmdListsProperties;
198200
};

test/conformance/exp_command_buffer/fixtures.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,7 @@ struct urUpdatableCommandBufferExpExecutionTest
113113
}
114114

115115
// Currently level zero driver doesn't support updating execution range.
116-
// Also disable immediate command lists because there is synchronization issue causing test failures.
117116
if (backend == UR_PLATFORM_BACKEND_LEVEL_ZERO) {
118-
setenv("UR_L0_USE_IMMEDIATE_COMMANDLISTS", "0", 0);
119117
updatable_execution_range_support = false;
120118
}
121119

0 commit comments

Comments
 (0)