Skip to content

Commit 00e8cf3

Browse files
RossBruntonkbenzie
authored andcommitted
Make invalid kernel arguments UB (#17962)
Instead of returning INVALID_KERNEL_ARGS, calling a kernel with invalid arguments is now UB. In addition, the Invalid kernel arguments enum itself is now deprecated (as it is no longer returned) and the OpenCL backend now writes a message to the log.
1 parent ab5ceee commit 00e8cf3

File tree

10 files changed

+21
-42
lines changed

10 files changed

+21
-42
lines changed

include/ur_api.h

Lines changed: 5 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

include/ur_print.hpp

Lines changed: 0 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

scripts/core/common.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ etors:
196196
- name: ERROR_INVALID_WORK_DIMENSION
197197
desc: "Invalid work dimension"
198198
- name: ERROR_INVALID_KERNEL_ARGS
199-
desc: "Invalid kernel args"
199+
desc: "[deprecated-value] No longer used - invalid kernel args are now UB"
200200
- name: ERROR_INVALID_KERNEL
201201
desc: "Invalid kernel"
202202
- name: ERROR_INVALID_KERNEL_NAME

scripts/core/enqueue.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ name: KernelLaunch
1919
ordinal: "0"
2020
analogue:
2121
- "**clEnqueueNDRangeKernel**"
22+
details:
23+
- "Providing invalid kernel arguments is Undefined Behavior."
2224
params:
2325
- type: $x_queue_handle_t
2426
name: hQueue
@@ -65,8 +67,6 @@ returns:
6567
- $X_RESULT_ERROR_INVALID_WORK_DIMENSION
6668
- $X_RESULT_ERROR_INVALID_WORK_GROUP_SIZE
6769
- $X_RESULT_ERROR_INVALID_VALUE
68-
- $X_RESULT_ERROR_INVALID_KERNEL_ARGS
69-
- "The kernel argument values have not been specified."
7070
- $X_RESULT_ERROR_OUT_OF_HOST_MEMORY
7171
- $X_RESULT_ERROR_OUT_OF_RESOURCES
7272
--- #--------------------------------------------------------------------------

source/adapters/opencl/common.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ ur_result_t mapCLErrorToUR(cl_int Result) {
8686
case CL_DEVICE_NOT_AVAILABLE:
8787
return UR_RESULT_ERROR_DEVICE_NOT_AVAILABLE;
8888
case CL_INVALID_KERNEL_ARGS:
89-
return UR_RESULT_ERROR_INVALID_KERNEL_ARGS;
89+
return UR_RESULT_ERROR_ADAPTER_SPECIFIC;
9090
case CL_INVALID_COMMAND_QUEUE:
9191
return UR_RESULT_ERROR_INVALID_QUEUE;
9292
case CL_INVALID_ARG_SIZE:

source/adapters/opencl/enqueue.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,16 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueKernelLaunch(
6565
cl_event Event;
6666
std::vector<cl_event> CLWaitEvents(numEventsInWaitList);
6767
MapUREventsToCL(numEventsInWaitList, phEventWaitList, CLWaitEvents);
68-
CL_RETURN_ON_FAILURE(clEnqueueNDRangeKernel(
68+
auto Err = clEnqueueNDRangeKernel(
6969
hQueue->CLQueue, hKernel->CLKernel, workDim, pGlobalWorkOffset,
7070
pGlobalWorkSize,
7171
compiledLocalWorksize.empty() ? pLocalWorkSize
7272
: compiledLocalWorksize.data(),
73-
numEventsInWaitList, CLWaitEvents.data(), ifUrEvent(phEvent, Event)));
73+
numEventsInWaitList, CLWaitEvents.data(), ifUrEvent(phEvent, Event));
74+
if (Err == CL_INVALID_KERNEL_ARGS) {
75+
ur::cl::getAdapter()->log.error("Kernel called with invalid arguments");
76+
}
77+
CL_RETURN_ON_FAILURE(Err);
7478

7579
UR_RETURN_ON_FAILURE(createUREvent(Event, hQueue->Context, hQueue, phEvent));
7680
return UR_RESULT_SUCCESS;

source/loader/ur_libapi.cpp

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

source/ur/ur.hpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,6 @@ static auto getUrResultString = [](ur_result_t Result) {
123123
return "UR_RESULT_ERROR_INVALID_WORK_ITEM_SIZE";
124124
case UR_RESULT_ERROR_INVALID_WORK_DIMENSION:
125125
return "UR_RESULT_ERROR_INVALID_WORK_DIMENSION";
126-
case UR_RESULT_ERROR_INVALID_KERNEL_ARGS:
127-
return "UR_RESULT_ERROR_INVALID_KERNEL_ARGS";
128126
case UR_RESULT_ERROR_INVALID_KERNEL:
129127
return "UR_RESULT_ERROR_INVALID_KERNEL";
130128
case UR_RESULT_ERROR_INVALID_KERNEL_NAME:

source/ur_api.cpp

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/conformance/enqueue/urEnqueueKernelLaunch.cpp

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -153,29 +153,6 @@ TEST_P(urEnqueueKernelLaunchTest, InvalidWorkGroupSize) {
153153
result == UR_RESULT_SUCCESS);
154154
}
155155

156-
TEST_P(urEnqueueKernelLaunchTest, InvalidKernelArgs) {
157-
// Cuda and hip both lack any way to validate kernel args
158-
UUR_KNOWN_FAILURE_ON(uur::CUDA{}, uur::HIP{});
159-
UUR_KNOWN_FAILURE_ON(uur::LevelZero{}, uur::LevelZeroV2{});
160-
161-
ur_platform_backend_t backend;
162-
ASSERT_SUCCESS(urPlatformGetInfo(platform, UR_PLATFORM_INFO_BACKEND,
163-
sizeof(ur_platform_backend_t), &backend,
164-
nullptr));
165-
166-
if (backend == UR_PLATFORM_BACKEND_CUDA ||
167-
backend == UR_PLATFORM_BACKEND_HIP ||
168-
backend == UR_PLATFORM_BACKEND_LEVEL_ZERO) {
169-
GTEST_FAIL() << "AMD, L0 and Nvidia can't check kernel arguments.";
170-
}
171-
172-
// Enqueue kernel without setting any args
173-
ASSERT_EQ_RESULT(urEnqueueKernelLaunch(queue, kernel, n_dimensions,
174-
&global_offset, &global_size, nullptr,
175-
0, nullptr, nullptr),
176-
UR_RESULT_ERROR_INVALID_KERNEL_ARGS);
177-
}
178-
179156
TEST_P(urEnqueueKernelLaunchKernelWgSizeTest, Success) {
180157
UUR_KNOWN_FAILURE_ON(uur::LevelZero{}, uur::LevelZeroV2{});
181158

0 commit comments

Comments
 (0)