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
fabiomestre committed Sep 9, 2024
1 parent 14bc901 commit 67c32dc
Show file tree
Hide file tree
Showing 14 changed files with 239 additions and 223 deletions.
8 changes: 3 additions & 5 deletions include/ur_api.h
View file
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
View file
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
View file
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
71 changes: 41 additions & 30 deletions source/adapters/hip/command_buffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "kernel.hpp"
#include "memory.hpp"
#include "queue.hpp"
#include <iostream>

#include <cstring>

Expand Down Expand Up @@ -330,9 +331,15 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferAppendKernelLaunchExp(
UR_RESULT_ERROR_INVALID_KERNEL);
UR_ASSERT(workDim > 0, UR_RESULT_ERROR_INVALID_WORK_DIMENSION);
UR_ASSERT(workDim < 4, UR_RESULT_ERROR_INVALID_WORK_DIMENSION);

UR_ASSERT(!(pSyncPointWaitList == NULL && numSyncPointsInWaitList > 0),
UR_RESULT_ERROR_INVALID_EVENT_WAIT_LIST);

for (uint32_t i = 0; i < numKernelAlternatives; ++i) {
UR_ASSERT(phKernelAlternatives[i] != hKernel,
UR_RESULT_ERROR_INVALID_VALUE);
}

hipGraphNode_t GraphNode;
std::vector<hipGraphNode_t> DepsList;

Expand Down Expand Up @@ -866,37 +873,41 @@ 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);
// const uint32_t NewWorkDim = UpdateCommandDesc->newWorkDim;
// if (!NewWorkDim && Command->Kernel != UpdateCommandDesc->hNewKernel) {
// return UR_RESULT_ERROR_INVALID_OPERATION;
// }

if (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;
}
// if (NewWorkDim) {
// UR_ASSERT(NewWorkDim > 0, UR_RESULT_ERROR_INVALID_WORK_DIMENSION);
// UR_ASSERT(NewWorkDim < 4, UR_RESULT_ERROR_INVALID_WORK_DIMENSION);

// 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();
std::cerr << "HERE" << std::endl;
std::cerr << UpdateCommandDesc->newWorkDim << std::endl;
std::cerr << Command->WorkDim << std::endl;

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 +918,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 +1031,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
43 changes: 23 additions & 20 deletions source/adapters/level_zero/command_buffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1320,35 +1320,40 @@ ur_result_t validateCommandDesc(
->mutableCommandFlags;
logger::debug("Mutable features supported by device {}", SupportedFeatures);

uint32_t Dim = CommandDesc->newWorkDim;
if (Dim != 0) {
// Error if work dim changes
if (Dim != Command->WorkDim) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}
// kernel handle updates are not yet supported.
if (CommandDesc->hNewKernel != Command->Kernel) {
return UR_RESULT_ERROR_UNSUPPORTED_FEATURE;
}

// Error If Local size and not global size
if ((CommandDesc->pNewLocalWorkSize != nullptr) &&
(CommandDesc->pNewGlobalWorkSize == nullptr)) {
// uint32_t Dim = CommandDesc->newWorkDim;
// if (Dim != 0) {
// Error if work dim changes
if (CommandDesc->hNewKernel == Command->Kernel && CommandDesc->newWorkDim != Command->WorkDim) {
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 = CommandDesc->pNewLocalWorkSize == nullptr;
const bool IsOriginalLocalSizeNull = !Command->UserDefinedLocalSize;
// // Error If Local size and not global size
// if ((CommandDesc->pNewLocalWorkSize != nullptr) &&
// (CommandDesc->pNewGlobalWorkSize == nullptr)) {
// return UR_RESULT_ERROR_INVALID_OPERATION;
// }

if (IsNewLocalSizeNull ^ IsOriginalLocalSizeNull) {
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 = CommandDesc->pNewLocalWorkSize == nullptr;
// const bool IsOriginalLocalSizeNull = !Command->UserDefinedLocalSize;
//
// if (IsNewLocalSizeNull ^ IsOriginalLocalSizeNull) {
// return UR_RESULT_ERROR_INVALID_OPERATION;
// }
// }

// 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 (NewGlobalWorkOffset) {
if (!CommandBuffer->Context->getPlatform()
->ZeDriverGlobalOffsetExtensionFound) {
logger::error("No global offset extension found on this driver");
Expand Down Expand Up @@ -1618,8 +1623,6 @@ ur_result_t urCommandBufferUpdateKernelLaunchExp(
ur_exp_command_buffer_command_handle_t Command,
const ur_exp_command_buffer_update_kernel_launch_desc_t *CommandDesc) {
UR_ASSERT(Command->Kernel, UR_RESULT_ERROR_INVALID_NULL_HANDLE);
UR_ASSERT(CommandDesc->newWorkDim <= 3,
UR_RESULT_ERROR_INVALID_WORK_DIMENSION);

// Lock command, kernel and command buffer for update.
std::scoped_lock<ur_shared_mutex, ur_shared_mutex, ur_shared_mutex> Guard(
Expand Down
10 changes: 10 additions & 0 deletions source/loader/layers/validation/ur_valddi.cpp
View file
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
View file
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
Loading

0 comments on commit 67c32dc

Please sign in to comment.