Skip to content

Commit 3059fa1

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 510a1d4 commit 3059fa1

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
@@ -95,7 +95,6 @@ ur_exp_command_buffer_handle_t_::ur_exp_command_buffer_handle_t_(
9595
ZeCommandListDesc(ZeDesc), ZeFencesList(), QueueProperties(),
9696
SyncPoints(), NextSyncPoint(0),
9797
IsUpdatable(Desc ? Desc->isUpdatable : false) {
98-
(void)Desc;
9998
urContextRetain(Context);
10099
urDeviceRetain(Device);
101100
}
@@ -1085,6 +1084,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp(
10851084
auto CommandBuffer = Command->CommandBuffer;
10861085
uint32_t Dim = CommandDesc->newWorkDim;
10871086
const void *NextDesc = nullptr;
1087+
auto SupportedFeatures =
1088+
Command->CommandBuffer->Device->ZeDeviceMutableCmdListsProperties
1089+
->mutableCommandFlags;
10881090

10891091
// We need the created descriptors to live till the point when
10901092
// zexCommandListUpdateMutableCommandsExp is called at the end of the
@@ -1100,6 +1102,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp(
11001102

11011103
// Check if new global offset is provided.
11021104
size_t *NewGlobalWorkOffset = CommandDesc->pNewGlobalWorkOffset;
1105+
UR_ASSERT(!NewGlobalWorkOffset ||
1106+
(SupportedFeatures & ZE_MUTABLE_COMMAND_EXP_FLAG_GLOBAL_OFFSET),
1107+
UR_RESULT_ERROR_UNSUPPORTED_FEATURE);
11031108
if (NewGlobalWorkOffset && Dim > 0) {
11041109
if (!CommandBuffer->Context->getPlatform()
11051110
->ZeDriverGlobalOffsetExtensionFound) {
@@ -1119,6 +1124,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp(
11191124

11201125
// Check if new group size is provided.
11211126
size_t *NewLocalWorkSize = CommandDesc->pNewLocalWorkSize;
1127+
UR_ASSERT(!NewLocalWorkSize ||
1128+
(SupportedFeatures & ZE_MUTABLE_COMMAND_EXP_FLAG_GROUP_SIZE),
1129+
UR_RESULT_ERROR_UNSUPPORTED_FEATURE);
11221130
if (NewLocalWorkSize && Dim > 0) {
11231131
auto MutableGroupSizeDesc =
11241132
std::make_unique<ZeStruct<ze_mutable_group_size_exp_desc_t>>();
@@ -1133,6 +1141,12 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp(
11331141

11341142
// Check if new global size is provided and we need to update group count.
11351143
size_t *NewGlobalWorkSize = CommandDesc->pNewGlobalWorkSize;
1144+
UR_ASSERT(!NewGlobalWorkSize ||
1145+
(SupportedFeatures & ZE_MUTABLE_COMMAND_EXP_FLAG_GROUP_COUNT),
1146+
UR_RESULT_ERROR_UNSUPPORTED_FEATURE);
1147+
UR_ASSERT(!(NewGlobalWorkSize && !NewLocalWorkSize) ||
1148+
(SupportedFeatures & ZE_MUTABLE_COMMAND_EXP_FLAG_GROUP_SIZE),
1149+
UR_RESULT_ERROR_UNSUPPORTED_FEATURE);
11361150
if (NewGlobalWorkSize && Dim > 0) {
11371151
ze_group_count_t ZeThreadGroupDimensions{1, 1, 1};
11381152
uint32_t WG[3];
@@ -1164,6 +1178,12 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp(
11641178
}
11651179
}
11661180

1181+
UR_ASSERT(
1182+
(!CommandDesc->numNewMemObjArgs && !CommandDesc->numNewPointerArgs &&
1183+
!CommandDesc->numNewValueArgs) ||
1184+
(SupportedFeatures & ZE_MUTABLE_COMMAND_EXP_FLAG_KERNEL_ARGUMENTS),
1185+
UR_RESULT_ERROR_UNSUPPORTED_FEATURE);
1186+
11671187
// Check if new memory object arguments are provided.
11681188
for (uint32_t NewMemObjArgNum = CommandDesc->numNewMemObjArgs;
11691189
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:
@@ -1129,6 +1143,15 @@ ur_result_t ur_device_handle_t_::initialize(int SubSubDeviceOrdinal,
11291143
(ZeDevice, &Count, &Properties));
11301144
};
11311145

1146+
ZeDeviceMutableCmdListsProperties.Compute =
1147+
[ZeDevice](
1148+
ZeStruct<ze_mutable_command_list_exp_properties_t> &Properties) {
1149+
ze_device_properties_t P;
1150+
P.stype = ZE_STRUCTURE_TYPE_DEVICE_PROPERTIES;
1151+
P.pNext = &Properties;
1152+
ZE_CALL_NOCHECK(zeDeviceGetProperties, (ZeDevice, &P));
1153+
};
1154+
11321155
ImmCommandListUsed = this->useImmediateCommandLists();
11331156

11341157
uint32_t numQueueGroups = 0;

source/adapters/level_zero/device.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,4 +192,6 @@ struct ur_device_handle_t_ : _ur_object {
192192
ZeCache<ZeStruct<ze_device_cache_properties_t>> ZeDeviceCacheProperties;
193193
ZeCache<ZeStruct<ze_device_ip_version_ext_t>> ZeDeviceIpVersionExt;
194194
ZeCache<struct ze_global_memsize> ZeGlobalMemSize;
195+
ZeCache<ZeStruct<ze_mutable_command_list_exp_properties_t>>
196+
ZeDeviceMutableCmdListsProperties;
195197
};

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)