Skip to content

Commit

Permalink
Update spec to make kernel validation optional
Browse files Browse the repository at this point in the history
Several adapters don't support validating kernel signatures when
enqueued. To handle this, we now allow urEnqueueKernelLaunch to return
`SUCCESS` even when parameters are invalid.

Some tests have also been updated. The CUDA adapter has also been
updated to handle invalid arguments.
  • Loading branch information
RossBrunton committed Feb 24, 2025
1 parent 59a1bab commit 3fc0d49
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 26 deletions.
10 changes: 8 additions & 2 deletions unified-runtime/include/ur_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -7431,6 +7431,11 @@ UR_APIEXPORT ur_result_t UR_APICALL urEventSetCallback(
///////////////////////////////////////////////////////////////////////////////
/// @brief Enqueue a command to execute a kernel
///
/// @details
/// - Adapters may perform validation on the number of arguments set to the
/// kernel, but are not required to do so and may return
/// `::UR_RESULT_SUCCESS` even for invalid invocations.
///
/// @remarks
/// _Analogues_
/// - **clEnqueueNDRangeKernel**
Expand Down Expand Up @@ -7458,8 +7463,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urEventSetCallback(
/// - ::UR_RESULT_ERROR_INVALID_WORK_DIMENSION
/// - ::UR_RESULT_ERROR_INVALID_WORK_GROUP_SIZE
/// - ::UR_RESULT_ERROR_INVALID_VALUE
/// - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGS - "The kernel argument values
/// have not been specified."
/// - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGS
/// + The kernel argument values have not been specified and the adapter
/// is able to detect this.
/// - ::UR_RESULT_ERROR_OUT_OF_HOST_MEMORY
/// - ::UR_RESULT_ERROR_OUT_OF_RESOURCES
UR_APIEXPORT ur_result_t UR_APICALL urEnqueueKernelLaunch(
Expand Down
7 changes: 5 additions & 2 deletions unified-runtime/scripts/core/enqueue.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ type: function
desc: "Enqueue a command to execute a kernel"
class: $xEnqueue
name: KernelLaunch
details:
- "Adapters may perform validation on the number of arguments set to the kernel, but are not required to do so and may
return `$X_RESULT_SUCCESS` even for invalid invocations."
ordinal: "0"
analogue:
- "**clEnqueueNDRangeKernel**"
Expand Down Expand Up @@ -65,8 +68,8 @@ returns:
- $X_RESULT_ERROR_INVALID_WORK_DIMENSION
- $X_RESULT_ERROR_INVALID_WORK_GROUP_SIZE
- $X_RESULT_ERROR_INVALID_VALUE
- $X_RESULT_ERROR_INVALID_KERNEL_ARGS
- "The kernel argument values have not been specified."
- $X_RESULT_ERROR_INVALID_KERNEL_ARGS:
- "The kernel argument values have not been specified and the adapter is able to detect this."
- $X_RESULT_ERROR_OUT_OF_HOST_MEMORY
- $X_RESULT_ERROR_OUT_OF_RESOURCES
--- #--------------------------------------------------------------------------
Expand Down
10 changes: 8 additions & 2 deletions unified-runtime/source/loader/ur_libapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4982,6 +4982,11 @@ ur_result_t UR_APICALL urEventSetCallback(
///////////////////////////////////////////////////////////////////////////////
/// @brief Enqueue a command to execute a kernel
///
/// @details
/// - Adapters may perform validation on the number of arguments set to the
/// kernel, but are not required to do so and may return
/// `::UR_RESULT_SUCCESS` even for invalid invocations.
///
/// @remarks
/// _Analogues_
/// - **clEnqueueNDRangeKernel**
Expand Down Expand Up @@ -5009,8 +5014,9 @@ ur_result_t UR_APICALL urEventSetCallback(
/// - ::UR_RESULT_ERROR_INVALID_WORK_DIMENSION
/// - ::UR_RESULT_ERROR_INVALID_WORK_GROUP_SIZE
/// - ::UR_RESULT_ERROR_INVALID_VALUE
/// - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGS - "The kernel argument values
/// have not been specified."
/// - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGS
/// + The kernel argument values have not been specified and the adapter
/// is able to detect this.
/// - ::UR_RESULT_ERROR_OUT_OF_HOST_MEMORY
/// - ::UR_RESULT_ERROR_OUT_OF_RESOURCES
ur_result_t UR_APICALL urEnqueueKernelLaunch(
Expand Down
10 changes: 8 additions & 2 deletions unified-runtime/source/ur_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4347,6 +4347,11 @@ ur_result_t UR_APICALL urEventSetCallback(
///////////////////////////////////////////////////////////////////////////////
/// @brief Enqueue a command to execute a kernel
///
/// @details
/// - Adapters may perform validation on the number of arguments set to the
/// kernel, but are not required to do so and may return
/// `::UR_RESULT_SUCCESS` even for invalid invocations.
///
/// @remarks
/// _Analogues_
/// - **clEnqueueNDRangeKernel**
Expand Down Expand Up @@ -4374,8 +4379,9 @@ ur_result_t UR_APICALL urEventSetCallback(
/// - ::UR_RESULT_ERROR_INVALID_WORK_DIMENSION
/// - ::UR_RESULT_ERROR_INVALID_WORK_GROUP_SIZE
/// - ::UR_RESULT_ERROR_INVALID_VALUE
/// - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGS - "The kernel argument values
/// have not been specified."
/// - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGS
/// + The kernel argument values have not been specified and the adapter
/// is able to detect this.
/// - ::UR_RESULT_ERROR_OUT_OF_HOST_MEMORY
/// - ::UR_RESULT_ERROR_OUT_OF_RESOURCES
ur_result_t UR_APICALL urEnqueueKernelLaunch(
Expand Down
29 changes: 11 additions & 18 deletions unified-runtime/test/conformance/enqueue/urEnqueueKernelLaunch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,26 +154,19 @@ TEST_P(urEnqueueKernelLaunchTest, InvalidWorkGroupSize) {
}

TEST_P(urEnqueueKernelLaunchTest, InvalidKernelArgs) {
// Cuda and hip both lack any way to validate kernel args
UUR_KNOWN_FAILURE_ON(uur::CUDA{}, uur::HIP{});
UUR_KNOWN_FAILURE_ON(uur::LevelZero{}, uur::LevelZeroV2{});

ur_platform_backend_t backend;
ASSERT_SUCCESS(urPlatformGetInfo(platform, UR_PLATFORM_INFO_BACKEND,
sizeof(ur_platform_backend_t), &backend,
nullptr));

if (backend == UR_PLATFORM_BACKEND_CUDA ||
backend == UR_PLATFORM_BACKEND_HIP ||
backend == UR_PLATFORM_BACKEND_LEVEL_ZERO) {
GTEST_FAIL() << "AMD, L0 and Nvidia can't check kernel arguments.";
}
// Seems to segfault
UUR_KNOWN_FAILURE_ON(uur::HIP{});
// cuLaunchKernel seems to be returning CUDA_ERROR_INVALID_VALUE which is
// converted to UR_RESULT_ERROR_INVALID_VALUE
// https://github.com/oneapi-src/unified-runtime/issues/2720
UUR_KNOWN_FAILURE_ON(uur::CUDA{});

// Enqueue kernel without setting any args
ASSERT_EQ_RESULT(urEnqueueKernelLaunch(queue, kernel, n_dimensions,
&global_offset, &global_size, nullptr,
0, nullptr, nullptr),
UR_RESULT_ERROR_INVALID_KERNEL_ARGS);
auto error =
urEnqueueKernelLaunch(queue, kernel, n_dimensions, &global_offset,
&global_size, nullptr, 0, nullptr, nullptr);
ASSERT_TRUE(error == UR_RESULT_ERROR_INVALID_KERNEL_ARGS ||
error == UR_RESULT_SUCCESS);
}

TEST_P(urEnqueueKernelLaunchKernelWgSizeTest, Success) {
Expand Down

0 comments on commit 3fc0d49

Please sign in to comment.