Skip to content

Commit f249b9b

Browse files
committed
Update spec to make kernel validation optional
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.
1 parent bf7a654 commit f249b9b

File tree

6 files changed

+46
-31
lines changed

6 files changed

+46
-31
lines changed

include/ur_api.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7423,6 +7423,11 @@ UR_APIEXPORT ur_result_t UR_APICALL urEventSetCallback(
74237423
///////////////////////////////////////////////////////////////////////////////
74247424
/// @brief Enqueue a command to execute a kernel
74257425
///
7426+
/// @details
7427+
/// - Adapters may perform validation on the number of arguments set to the
7428+
/// kernel, but are not required to do so and may return
7429+
/// `::UR_RESULT_SUCCESS` even for invalid invocations.
7430+
///
74267431
/// @remarks
74277432
/// _Analogues_
74287433
/// - **clEnqueueNDRangeKernel**
@@ -7450,8 +7455,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urEventSetCallback(
74507455
/// - ::UR_RESULT_ERROR_INVALID_WORK_DIMENSION
74517456
/// - ::UR_RESULT_ERROR_INVALID_WORK_GROUP_SIZE
74527457
/// - ::UR_RESULT_ERROR_INVALID_VALUE
7453-
/// - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGS - "The kernel argument values
7454-
/// have not been specified."
7458+
/// - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGS
7459+
/// + The kernel argument values have not been specified and the adapter
7460+
/// is able to detect this.
74557461
/// - ::UR_RESULT_ERROR_OUT_OF_HOST_MEMORY
74567462
/// - ::UR_RESULT_ERROR_OUT_OF_RESOURCES
74577463
UR_APIEXPORT ur_result_t UR_APICALL urEnqueueKernelLaunch(

scripts/core/enqueue.yml

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ type: function
1616
desc: "Enqueue a command to execute a kernel"
1717
class: $xEnqueue
1818
name: KernelLaunch
19+
details:
20+
- "Adapters may perform validation on the number of arguments set to the kernel, but are not required to do so and may
21+
return `$X_RESULT_SUCCESS` even for invalid invocations."
1922
ordinal: "0"
2023
analogue:
2124
- "**clEnqueueNDRangeKernel**"
@@ -65,8 +68,8 @@ returns:
6568
- $X_RESULT_ERROR_INVALID_WORK_DIMENSION
6669
- $X_RESULT_ERROR_INVALID_WORK_GROUP_SIZE
6770
- $X_RESULT_ERROR_INVALID_VALUE
68-
- $X_RESULT_ERROR_INVALID_KERNEL_ARGS
69-
- "The kernel argument values have not been specified."
71+
- $X_RESULT_ERROR_INVALID_KERNEL_ARGS:
72+
- "The kernel argument values have not been specified and the adapter is able to detect this."
7073
- $X_RESULT_ERROR_OUT_OF_HOST_MEMORY
7174
- $X_RESULT_ERROR_OUT_OF_RESOURCES
7275
--- #--------------------------------------------------------------------------

source/adapters/cuda/enqueue.cpp

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -493,10 +493,18 @@ enqueueKernelLaunch(ur_queue_handle_t hQueue, ur_kernel_handle_t hKernel,
493493
}
494494

495495
auto &ArgIndices = hKernel->getArgIndices();
496-
UR_CHECK_ERROR(cuLaunchKernel(
497-
CuFunc, BlocksPerGrid[0], BlocksPerGrid[1], BlocksPerGrid[2],
498-
ThreadsPerBlock[0], ThreadsPerBlock[1], ThreadsPerBlock[2], LocalSize,
499-
CuStream, const_cast<void **>(ArgIndices.data()), nullptr));
496+
try {
497+
UR_CHECK_ERROR(cuLaunchKernel(
498+
CuFunc, BlocksPerGrid[0], BlocksPerGrid[1], BlocksPerGrid[2],
499+
ThreadsPerBlock[0], ThreadsPerBlock[1], ThreadsPerBlock[2], LocalSize,
500+
CuStream, const_cast<void **>(ArgIndices.data()), nullptr));
501+
} catch (ur_result Err) {
502+
// Cuda returns UR_RESULT_ERROR_INVALID_VALUE if the args are incorrect
503+
if (Err == UR_RESULT_ERROR_INVALID_VALUE) {
504+
return UR_RESULT_ERROR_INVALID_KERNEL_ARGS;
505+
}
506+
return Err;
507+
}
500508

501509
if (phEvent) {
502510
UR_CHECK_ERROR(RetImplEvent->record());

source/loader/ur_libapi.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4979,6 +4979,11 @@ ur_result_t UR_APICALL urEventSetCallback(
49794979
///////////////////////////////////////////////////////////////////////////////
49804980
/// @brief Enqueue a command to execute a kernel
49814981
///
4982+
/// @details
4983+
/// - Adapters may perform validation on the number of arguments set to the
4984+
/// kernel, but are not required to do so and may return
4985+
/// `::UR_RESULT_SUCCESS` even for invalid invocations.
4986+
///
49824987
/// @remarks
49834988
/// _Analogues_
49844989
/// - **clEnqueueNDRangeKernel**
@@ -5006,8 +5011,9 @@ ur_result_t UR_APICALL urEventSetCallback(
50065011
/// - ::UR_RESULT_ERROR_INVALID_WORK_DIMENSION
50075012
/// - ::UR_RESULT_ERROR_INVALID_WORK_GROUP_SIZE
50085013
/// - ::UR_RESULT_ERROR_INVALID_VALUE
5009-
/// - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGS - "The kernel argument values
5010-
/// have not been specified."
5014+
/// - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGS
5015+
/// + The kernel argument values have not been specified and the adapter
5016+
/// is able to detect this.
50115017
/// - ::UR_RESULT_ERROR_OUT_OF_HOST_MEMORY
50125018
/// - ::UR_RESULT_ERROR_OUT_OF_RESOURCES
50135019
ur_result_t UR_APICALL urEnqueueKernelLaunch(

source/ur_api.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4344,6 +4344,11 @@ ur_result_t UR_APICALL urEventSetCallback(
43444344
///////////////////////////////////////////////////////////////////////////////
43454345
/// @brief Enqueue a command to execute a kernel
43464346
///
4347+
/// @details
4348+
/// - Adapters may perform validation on the number of arguments set to the
4349+
/// kernel, but are not required to do so and may return
4350+
/// `::UR_RESULT_SUCCESS` even for invalid invocations.
4351+
///
43474352
/// @remarks
43484353
/// _Analogues_
43494354
/// - **clEnqueueNDRangeKernel**
@@ -4371,8 +4376,9 @@ ur_result_t UR_APICALL urEventSetCallback(
43714376
/// - ::UR_RESULT_ERROR_INVALID_WORK_DIMENSION
43724377
/// - ::UR_RESULT_ERROR_INVALID_WORK_GROUP_SIZE
43734378
/// - ::UR_RESULT_ERROR_INVALID_VALUE
4374-
/// - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGS - "The kernel argument values
4375-
/// have not been specified."
4379+
/// - ::UR_RESULT_ERROR_INVALID_KERNEL_ARGS
4380+
/// + The kernel argument values have not been specified and the adapter
4381+
/// is able to detect this.
43764382
/// - ::UR_RESULT_ERROR_OUT_OF_HOST_MEMORY
43774383
/// - ::UR_RESULT_ERROR_OUT_OF_RESOURCES
43784384
ur_result_t UR_APICALL urEnqueueKernelLaunch(

test/conformance/enqueue/urEnqueueKernelLaunch.cpp

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -141,26 +141,12 @@ TEST_P(urEnqueueKernelLaunchTest, InvalidWorkGroupSize) {
141141
}
142142

143143
TEST_P(urEnqueueKernelLaunchTest, InvalidKernelArgs) {
144-
// Cuda and hip both lack any way to validate kernel args
145-
UUR_KNOWN_FAILURE_ON(uur::CUDA{}, uur::HIP{});
146-
UUR_KNOWN_FAILURE_ON(uur::LevelZero{}, uur::LevelZeroV2{});
147-
148-
ur_platform_backend_t backend;
149-
ASSERT_SUCCESS(urPlatformGetInfo(platform, UR_PLATFORM_INFO_BACKEND,
150-
sizeof(ur_platform_backend_t), &backend,
151-
nullptr));
152-
153-
if (backend == UR_PLATFORM_BACKEND_CUDA ||
154-
backend == UR_PLATFORM_BACKEND_HIP ||
155-
backend == UR_PLATFORM_BACKEND_LEVEL_ZERO) {
156-
GTEST_FAIL() << "AMD, L0 and Nvidia can't check kernel arguments.";
157-
}
158-
159144
// Enqueue kernel without setting any args
160-
ASSERT_EQ_RESULT(urEnqueueKernelLaunch(queue, kernel, n_dimensions,
161-
&global_offset, &global_size, nullptr,
162-
0, nullptr, nullptr),
163-
UR_RESULT_ERROR_INVALID_KERNEL_ARGS);
145+
auto error =
146+
urEnqueueKernelLaunch(queue, kernel, n_dimensions, &global_offset,
147+
&global_size, nullptr, 0, nullptr, nullptr);
148+
ASSERT_TRUE(error == UR_RESULT_ERROR_INVALID_KERNEL_ARGS ||
149+
error == UR_RESULT_SUCCESS);
164150
}
165151

166152
TEST_P(urEnqueueKernelLaunchKernelWgSizeTest, Success) {

0 commit comments

Comments
 (0)