Skip to content

Commit

Permalink
Make newWorkDim non-optional and remove newLocalWorkgroup nullptr errors
Browse files Browse the repository at this point in the history
  • Loading branch information
fabiomestre committed Sep 9, 2024
1 parent 14bc901 commit 2e53a26
Show file tree
Hide file tree
Showing 13 changed files with 209 additions and 207 deletions.
8 changes: 3 additions & 5 deletions include/ur_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -8950,17 +8950,15 @@ urCommandBufferReleaseCommandExp(
/// - ::UR_RESULT_ERROR_INVALID_OPERATION
/// + If ::ur_exp_command_buffer_desc_t::isUpdatable was not set to true on creation of the command buffer `hCommand` belongs to.
/// + If the command-buffer `hCommand` belongs to has not been finalized.
/// + If `pUpdateKernellaunch->hNewKernel` is different from the currently active kernel in `hCommand`, and `pUpdateKernellaunch->newWorkDim` is zero.
/// + If `pUpdateKernellaunch->hNewKernel` is equal to the currently active kernel in `hCommand`, and `pUpdateKernellaunch->newWorkDim` is non-zero and different from the work-dim currently associated with `hCommand`.
/// + If `pUpdateKernellaunch->newWorkDim` is non-zero, and `pUpdateKernelLaunch->pNewLocalWorkSize` is set to a non-NULL value, and `pUpdateKernelLaunch->pNewGlobalWorkSize` is NULL.
/// + If `pUpdateKernellaunch->hNewKernel` is equal to the current kernel associated with `hCommand`, and `pUpdateKernellaunch->newWorkDim` is non-zero, and `pUpdateKernelLaunch->pNewLocalWorkSize` is set to a non-NULL value while `hCommand` is currently associated with a NULL local work size.
/// + If `pUpdateKernellaunch->hNewKernel` is equal to the current kernel associated with `hCommand`, and `pUpdateKernellaunch->newWorkDim` is non-zero, and `pUpdateKernelLaunch->pNewLocalWorkSize` is set to a NULL value while `hCommand` is currently associated with a non-NULL local work size.
/// + `pUpdateKernelLaunch->pNewLocalWorkSize != NULL && pUpdateKernelLaunch->pNewGlobalWorkSize == NULL`
/// + If `pUpdateKernellaunch->hNewKernel` is equal to the currently active kernel in `hCommand`, and `pUpdateKernellaunch->newWorkDim` is different from the work-dim currently associated with `hCommand`.
/// - ::UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_COMMAND_HANDLE_EXP
/// - ::UR_RESULT_ERROR_INVALID_MEM_OBJECT
/// - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGUMENT_INDEX
/// - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGUMENT_SIZE
/// - ::UR_RESULT_ERROR_INVALID_ENUMERATION
/// - ::UR_RESULT_ERROR_INVALID_WORK_DIMENSION
/// + `pUpdateKernelLaunch->newWorkDim < 0 || pUpdateKernelLaunch->newWorkDim > 3`
/// - ::UR_RESULT_ERROR_INVALID_WORK_GROUP_SIZE
/// - ::UR_RESULT_ERROR_INVALID_VALUE
/// + If `pUpdateKernelLaunch->hNewKernel` was not passed to the `hKernel` or `phKernelAlternatives` parameters of ::urCommandBufferAppendKernelLaunchExp when this command was created.
Expand Down
10 changes: 4 additions & 6 deletions scripts/core/exp-command-buffer.yml
Original file line number Diff line number Diff line change
Expand Up @@ -954,17 +954,15 @@ returns:
- $X_RESULT_ERROR_INVALID_OPERATION:
- "If $x_exp_command_buffer_desc_t::isUpdatable was not set to true on creation of the command buffer `hCommand` belongs to."
- "If the command-buffer `hCommand` belongs to has not been finalized."
- "If `pUpdateKernellaunch->hNewKernel` is different from the currently active kernel in `hCommand`, and `pUpdateKernellaunch->newWorkDim` is zero."
- "If `pUpdateKernellaunch->hNewKernel` is equal to the currently active kernel in `hCommand`, and `pUpdateKernellaunch->newWorkDim` is non-zero and different from the work-dim currently associated with `hCommand`."
- "If `pUpdateKernellaunch->newWorkDim` is non-zero, and `pUpdateKernelLaunch->pNewLocalWorkSize` is set to a non-NULL value, and `pUpdateKernelLaunch->pNewGlobalWorkSize` is NULL."
- "If `pUpdateKernellaunch->hNewKernel` is equal to the current kernel associated with `hCommand`, and `pUpdateKernellaunch->newWorkDim` is non-zero, and `pUpdateKernelLaunch->pNewLocalWorkSize` is set to a non-NULL value while `hCommand` is currently associated with a NULL local work size."
- "If `pUpdateKernellaunch->hNewKernel` is equal to the current kernel associated with `hCommand`, and `pUpdateKernellaunch->newWorkDim` is non-zero, and `pUpdateKernelLaunch->pNewLocalWorkSize` is set to a NULL value while `hCommand` is currently associated with a non-NULL local work size."
- "`pUpdateKernelLaunch->pNewLocalWorkSize != NULL && pUpdateKernelLaunch->pNewGlobalWorkSize == NULL`"
- "If `pUpdateKernellaunch->hNewKernel` is equal to the currently active kernel in `hCommand`, and `pUpdateKernellaunch->newWorkDim` is different from the work-dim currently associated with `hCommand`."
- $X_RESULT_ERROR_INVALID_COMMAND_BUFFER_COMMAND_HANDLE_EXP
- $X_RESULT_ERROR_INVALID_MEM_OBJECT
- $X_RESULT_ERROR_INVALID_KERNEL_ARGUMENT_INDEX
- $X_RESULT_ERROR_INVALID_KERNEL_ARGUMENT_SIZE
- $X_RESULT_ERROR_INVALID_ENUMERATION
- $X_RESULT_ERROR_INVALID_WORK_DIMENSION
- $X_RESULT_ERROR_INVALID_WORK_DIMENSION:
- "`pUpdateKernelLaunch->newWorkDim < 0 || pUpdateKernelLaunch->newWorkDim > 3`"
- $X_RESULT_ERROR_INVALID_WORK_GROUP_SIZE
- $X_RESULT_ERROR_INVALID_VALUE:
- "If `pUpdateKernelLaunch->hNewKernel` was not passed to the `hKernel` or `phKernelAlternatives` parameters of $xCommandBufferAppendKernelLaunchExp when this command was created."
Expand Down
54 changes: 27 additions & 27 deletions source/adapters/cuda/command_buffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -887,37 +887,37 @@ validateCommandDesc(ur_exp_command_buffer_command_handle_t Command,
return UR_RESULT_ERROR_INVALID_OPERATION;
}

const uint32_t NewWorkDim = UpdateCommandDesc->newWorkDim;
if (!NewWorkDim && Command->Kernel != UpdateCommandDesc->hNewKernel) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}

if (NewWorkDim) {
UR_ASSERT(NewWorkDim > 0, UR_RESULT_ERROR_INVALID_WORK_DIMENSION);
UR_ASSERT(NewWorkDim < 4, UR_RESULT_ERROR_INVALID_WORK_DIMENSION);

if (NewWorkDim != Command->WorkDim &&
Command->Kernel == UpdateCommandDesc->hNewKernel) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}
// const uint32_t NewWorkDim = UpdateCommandDesc->newWorkDim;
// if (!NewWorkDim) {
// return UR_RESULT_ERROR_INVALID_OPERATION;
// }

// Error If Local size and not global size
if ((UpdateCommandDesc->pNewLocalWorkSize != nullptr) &&
(UpdateCommandDesc->pNewGlobalWorkSize == nullptr)) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}

// Error if local size non-nullptr and created with null
// or if local size nullptr and created with non-null
const bool IsNewLocalSizeNull =
UpdateCommandDesc->pNewLocalWorkSize == nullptr;
const bool IsOriginalLocalSizeNull = Command->isNullLocalSize();
// if (NewWorkDim) {
// UR_ASSERT(NewWorkDim > 0, UR_RESULT_ERROR_INVALID_WORK_DIMENSION);
// UR_ASSERT(NewWorkDim < 4, UR_RESULT_ERROR_INVALID_WORK_DIMENSION);

if (IsNewLocalSizeNull ^ IsOriginalLocalSizeNull) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}
if (UpdateCommandDesc->newWorkDim != Command->WorkDim &&
Command->Kernel == UpdateCommandDesc->hNewKernel) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}

// // Error If Local size and not global size
// if ((UpdateCommandDesc->pNewLocalWorkSize != nullptr) &&
// (UpdateCommandDesc->pNewGlobalWorkSize == nullptr)) {
// return UR_RESULT_ERROR_INVALID_OPERATION;
// }

// // Error if local size non-nullptr and created with null
// // or if local size nullptr and created with non-null
// const bool IsNewLocalSizeNull =
// UpdateCommandDesc->pNewLocalWorkSize == nullptr;
// const bool IsOriginalLocalSizeNull = Command->isNullLocalSize();
//
// if (IsNewLocalSizeNull ^ IsOriginalLocalSizeNull) {
// return UR_RESULT_ERROR_INVALID_OPERATION;
// }
// }

if (!Command->ValidKernelHandles.count(UpdateCommandDesc->hNewKernel)) {
return UR_RESULT_ERROR_INVALID_VALUE;
}
Expand Down
68 changes: 34 additions & 34 deletions source/adapters/hip/command_buffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ commandHandleReleaseInternal(ur_exp_command_buffer_command_handle_t Command) {

ur_exp_command_buffer_handle_t_::ur_exp_command_buffer_handle_t_(
ur_context_handle_t hContext, ur_device_handle_t hDevice, bool IsUpdatable)
: Context(hContext), Device(hDevice), IsUpdatable(IsUpdatable),
HIPGraph{nullptr}, HIPGraphExec{nullptr}, RefCountInternal{1},
RefCountExternal{1}, NextSyncPoint{0} {
: Context(hContext), Device(hDevice),
IsUpdatable(IsUpdatable), HIPGraph{nullptr}, HIPGraphExec{nullptr},
RefCountInternal{1}, RefCountExternal{1}, NextSyncPoint{0} {
urContextRetain(hContext);
urDeviceRetain(hDevice);
}
Expand Down Expand Up @@ -866,37 +866,37 @@ validateCommandDesc(ur_exp_command_buffer_command_handle_t Command,
return UR_RESULT_ERROR_INVALID_OPERATION;
}

const uint32_t NewWorkDim = UpdateCommandDesc->newWorkDim;
if (!NewWorkDim && Command->Kernel != UpdateCommandDesc->hNewKernel) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}

if (NewWorkDim) {
UR_ASSERT(NewWorkDim > 0, UR_RESULT_ERROR_INVALID_WORK_DIMENSION);
UR_ASSERT(NewWorkDim < 4, UR_RESULT_ERROR_INVALID_WORK_DIMENSION);

if (NewWorkDim != Command->WorkDim &&
Command->Kernel == UpdateCommandDesc->hNewKernel) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}
// const uint32_t NewWorkDim = UpdateCommandDesc->newWorkDim;
// if (!NewWorkDim && Command->Kernel != UpdateCommandDesc->hNewKernel) {
// return UR_RESULT_ERROR_INVALID_OPERATION;
// }

// Error If Local size and not global size
if ((UpdateCommandDesc->pNewLocalWorkSize != nullptr) &&
(UpdateCommandDesc->pNewGlobalWorkSize == nullptr)) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}

// Error if local size non-nullptr and created with null
// or if local size nullptr and created with non-null
const bool IsNewLocalSizeNull =
UpdateCommandDesc->pNewLocalWorkSize == nullptr;
const bool IsOriginalLocalSizeNull = Command->isNullLocalSize();
// if (NewWorkDim) {
// UR_ASSERT(NewWorkDim > 0, UR_RESULT_ERROR_INVALID_WORK_DIMENSION);
// UR_ASSERT(NewWorkDim < 4, UR_RESULT_ERROR_INVALID_WORK_DIMENSION);

if (IsNewLocalSizeNull ^ IsOriginalLocalSizeNull) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}
if (UpdateCommandDesc->newWorkDim != Command->WorkDim &&
Command->Kernel == UpdateCommandDesc->hNewKernel) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}

// // Error If Local size and not global size
// if ((UpdateCommandDesc->pNewLocalWorkSize != nullptr) &&
// (UpdateCommandDesc->pNewGlobalWorkSize == nullptr)) {
// return UR_RESULT_ERROR_INVALID_OPERATION;
// }

// // Error if local size non-nullptr and created with null
// // or if local size nullptr and created with non-null
// const bool IsNewLocalSizeNull =
// UpdateCommandDesc->pNewLocalWorkSize == nullptr;
// const bool IsOriginalLocalSizeNull = Command->isNullLocalSize();
//
// if (IsNewLocalSizeNull ^ IsOriginalLocalSizeNull) {
// return UR_RESULT_ERROR_INVALID_OPERATION;
// }
// }

if (!Command->ValidKernelHandles.count(UpdateCommandDesc->hNewKernel)) {
return UR_RESULT_ERROR_INVALID_VALUE;
}
Expand All @@ -907,8 +907,8 @@ validateCommandDesc(ur_exp_command_buffer_command_handle_t Command,
/**
* Updates the arguments of CommandDesc->hNewKernel
* @param[in] Device The device associated with the kernel being updated.
* @param[in] UpdateCommandDesc The update command description that contains the
* new kernel and its arguments.
* @param[in] UpdateCommandDesc The update command description that contains
* the new kernel and its arguments.
* @return UR_RESULT_SUCCESS or an error code on failure
*/
ur_result_t
Expand Down Expand Up @@ -1020,8 +1020,8 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp(
updateKernelArguments(CommandBuffer->Device, pUpdateKernelLaunch));
UR_CHECK_ERROR(updateCommand(hCommand, pUpdateKernelLaunch));

// If no worksize is provided make sure we pass nullptr to setKernelParams so
// it can guess the local work size.
// If no worksize is provided make sure we pass nullptr to setKernelParams
// so it can guess the local work size.
const bool ProvidedLocalSize = !hCommand->isNullLocalSize();
size_t *LocalWorkSize = ProvidedLocalSize ? hCommand->LocalWorkSize : nullptr;

Expand Down
10 changes: 10 additions & 0 deletions source/loader/layers/validation/ur_valddi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8955,6 +8955,16 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferUpdateKernelLaunchExp(
if (NULL == pUpdateKernelLaunch) {
return UR_RESULT_ERROR_INVALID_NULL_POINTER;
}

if (pUpdateKernelLaunch->pNewLocalWorkSize != NULL &&
pUpdateKernelLaunch->pNewGlobalWorkSize == NULL) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}

if (pUpdateKernelLaunch->newWorkDim < 0 ||
pUpdateKernelLaunch->newWorkDim > 3) {
return UR_RESULT_ERROR_INVALID_WORK_DIMENSION;
}
}

ur_result_t result =
Expand Down
8 changes: 3 additions & 5 deletions source/loader/ur_libapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8314,17 +8314,15 @@ ur_result_t UR_APICALL urCommandBufferReleaseCommandExp(
/// - ::UR_RESULT_ERROR_INVALID_OPERATION
/// + If ::ur_exp_command_buffer_desc_t::isUpdatable was not set to true on creation of the command buffer `hCommand` belongs to.
/// + If the command-buffer `hCommand` belongs to has not been finalized.
/// + If `pUpdateKernellaunch->hNewKernel` is different from the currently active kernel in `hCommand`, and `pUpdateKernellaunch->newWorkDim` is zero.
/// + If `pUpdateKernellaunch->hNewKernel` is equal to the currently active kernel in `hCommand`, and `pUpdateKernellaunch->newWorkDim` is non-zero and different from the work-dim currently associated with `hCommand`.
/// + If `pUpdateKernellaunch->newWorkDim` is non-zero, and `pUpdateKernelLaunch->pNewLocalWorkSize` is set to a non-NULL value, and `pUpdateKernelLaunch->pNewGlobalWorkSize` is NULL.
/// + If `pUpdateKernellaunch->hNewKernel` is equal to the current kernel associated with `hCommand`, and `pUpdateKernellaunch->newWorkDim` is non-zero, and `pUpdateKernelLaunch->pNewLocalWorkSize` is set to a non-NULL value while `hCommand` is currently associated with a NULL local work size.
/// + If `pUpdateKernellaunch->hNewKernel` is equal to the current kernel associated with `hCommand`, and `pUpdateKernellaunch->newWorkDim` is non-zero, and `pUpdateKernelLaunch->pNewLocalWorkSize` is set to a NULL value while `hCommand` is currently associated with a non-NULL local work size.
/// + `pUpdateKernelLaunch->pNewLocalWorkSize != NULL && pUpdateKernelLaunch->pNewGlobalWorkSize == NULL`
/// + If `pUpdateKernellaunch->hNewKernel` is equal to the currently active kernel in `hCommand`, and `pUpdateKernellaunch->newWorkDim` is different from the work-dim currently associated with `hCommand`.
/// - ::UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_COMMAND_HANDLE_EXP
/// - ::UR_RESULT_ERROR_INVALID_MEM_OBJECT
/// - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGUMENT_INDEX
/// - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGUMENT_SIZE
/// - ::UR_RESULT_ERROR_INVALID_ENUMERATION
/// - ::UR_RESULT_ERROR_INVALID_WORK_DIMENSION
/// + `pUpdateKernelLaunch->newWorkDim < 0 || pUpdateKernelLaunch->newWorkDim > 3`
/// - ::UR_RESULT_ERROR_INVALID_WORK_GROUP_SIZE
/// - ::UR_RESULT_ERROR_INVALID_VALUE
/// + If `pUpdateKernelLaunch->hNewKernel` was not passed to the `hKernel` or `phKernelAlternatives` parameters of ::urCommandBufferAppendKernelLaunchExp when this command was created.
Expand Down
8 changes: 3 additions & 5 deletions source/ur_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7027,17 +7027,15 @@ ur_result_t UR_APICALL urCommandBufferReleaseCommandExp(
/// - ::UR_RESULT_ERROR_INVALID_OPERATION
/// + If ::ur_exp_command_buffer_desc_t::isUpdatable was not set to true on creation of the command buffer `hCommand` belongs to.
/// + If the command-buffer `hCommand` belongs to has not been finalized.
/// + If `pUpdateKernellaunch->hNewKernel` is different from the currently active kernel in `hCommand`, and `pUpdateKernellaunch->newWorkDim` is zero.
/// + If `pUpdateKernellaunch->hNewKernel` is equal to the currently active kernel in `hCommand`, and `pUpdateKernellaunch->newWorkDim` is non-zero and different from the work-dim currently associated with `hCommand`.
/// + If `pUpdateKernellaunch->newWorkDim` is non-zero, and `pUpdateKernelLaunch->pNewLocalWorkSize` is set to a non-NULL value, and `pUpdateKernelLaunch->pNewGlobalWorkSize` is NULL.
/// + If `pUpdateKernellaunch->hNewKernel` is equal to the current kernel associated with `hCommand`, and `pUpdateKernellaunch->newWorkDim` is non-zero, and `pUpdateKernelLaunch->pNewLocalWorkSize` is set to a non-NULL value while `hCommand` is currently associated with a NULL local work size.
/// + If `pUpdateKernellaunch->hNewKernel` is equal to the current kernel associated with `hCommand`, and `pUpdateKernellaunch->newWorkDim` is non-zero, and `pUpdateKernelLaunch->pNewLocalWorkSize` is set to a NULL value while `hCommand` is currently associated with a non-NULL local work size.
/// + `pUpdateKernelLaunch->pNewLocalWorkSize != NULL && pUpdateKernelLaunch->pNewGlobalWorkSize == NULL`
/// + If `pUpdateKernellaunch->hNewKernel` is equal to the currently active kernel in `hCommand`, and `pUpdateKernellaunch->newWorkDim` is different from the work-dim currently associated with `hCommand`.
/// - ::UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_COMMAND_HANDLE_EXP
/// - ::UR_RESULT_ERROR_INVALID_MEM_OBJECT
/// - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGUMENT_INDEX
/// - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGUMENT_SIZE
/// - ::UR_RESULT_ERROR_INVALID_ENUMERATION
/// - ::UR_RESULT_ERROR_INVALID_WORK_DIMENSION
/// + `pUpdateKernelLaunch->newWorkDim < 0 || pUpdateKernelLaunch->newWorkDim > 3`
/// - ::UR_RESULT_ERROR_INVALID_WORK_GROUP_SIZE
/// - ::UR_RESULT_ERROR_INVALID_VALUE
/// + If `pUpdateKernelLaunch->hNewKernel` was not passed to the `hKernel` or `phKernelAlternatives` parameters of ::urCommandBufferAppendKernelLaunchExp when this command was created.
Expand Down
Loading

0 comments on commit 2e53a26

Please sign in to comment.