Skip to content

Commit dfefafd

Browse files
committed
[UR] Make invalid kernel arguments UB
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 585606a commit dfefafd

File tree

12 files changed

+22
-51
lines changed

12 files changed

+22
-51
lines changed

sycl/source/detail/error_handling/error_handling.cpp

-8
Original file line numberDiff line numberDiff line change
@@ -404,14 +404,6 @@ void handleErrorOrWarning(ur_result_t Error, const device_impl &DeviceImpl,
404404
case UR_RESULT_ERROR_INVALID_WORK_GROUP_SIZE:
405405
return handleInvalidWorkGroupSize(DeviceImpl, Kernel, NDRDesc);
406406

407-
case UR_RESULT_ERROR_INVALID_KERNEL_ARGS:
408-
throw detail::set_ur_error(
409-
sycl::exception(
410-
make_error_code(errc::kernel_argument),
411-
"The kernel argument values have not been specified OR a kernel "
412-
"argument declared to be a pointer to a type."),
413-
UR_RESULT_ERROR_INVALID_KERNEL_ARGS);
414-
415407
case UR_RESULT_ERROR_INVALID_WORK_ITEM_SIZE:
416408
return handleInvalidWorkItemSize(DeviceImpl, NDRDesc);
417409

sycl/source/exception.cpp

-1
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,6 @@ __SYCL_EXPORT const char *stringifyErrorCode(int32_t error) {
103103
_UR_ERRC(UR_RESULT_ERROR_INVALID_DEVICE_PARTITION_COUNT)
104104
_UR_ERRC(UR_RESULT_ERROR_INVALID_WORK_ITEM_SIZE)
105105
_UR_ERRC(UR_RESULT_ERROR_INVALID_WORK_DIMENSION)
106-
_UR_ERRC(UR_RESULT_ERROR_INVALID_KERNEL_ARGS)
107106
_UR_ERRC(UR_RESULT_ERROR_INVALID_KERNEL)
108107
_UR_ERRC(UR_RESULT_ERROR_INVALID_KERNEL_NAME)
109108
_UR_ERRC(UR_RESULT_ERROR_INVALID_KERNEL_ARGUMENT_INDEX)

unified-runtime/include/ur_api.h

+5-4
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

unified-runtime/include/ur_print.hpp

-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

unified-runtime/scripts/core/common.yml

+1-1
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

unified-runtime/scripts/core/enqueue.yml

+2-2
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
--- #--------------------------------------------------------------------------

unified-runtime/source/adapters/opencl/common.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ ur_result_t mapCLErrorToUR(cl_int Result) {
9494
case CL_DEVICE_NOT_AVAILABLE:
9595
return UR_RESULT_ERROR_DEVICE_NOT_AVAILABLE;
9696
case CL_INVALID_KERNEL_ARGS:
97-
return UR_RESULT_ERROR_INVALID_KERNEL_ARGS;
97+
return UR_RESULT_ERROR_ADAPTER_SPECIFIC;
9898
case CL_INVALID_COMMAND_QUEUE:
9999
return UR_RESULT_ERROR_INVALID_QUEUE;
100100
case CL_INVALID_ARG_SIZE:

unified-runtime/source/adapters/opencl/enqueue.cpp

+7-2
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
//
99
//===----------------------------------------------------------------------===//
1010

11+
#include "adapter.hpp"
1112
#include "common.hpp"
1213
#include "context.hpp"
1314
#include "event.hpp"
@@ -64,12 +65,16 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueKernelLaunch(
6465
cl_event Event;
6566
std::vector<cl_event> CLWaitEvents(numEventsInWaitList);
6667
MapUREventsToCL(numEventsInWaitList, phEventWaitList, CLWaitEvents);
67-
CL_RETURN_ON_FAILURE(clEnqueueNDRangeKernel(
68+
auto Err = clEnqueueNDRangeKernel(
6869
hQueue->CLQueue, hKernel->CLKernel, workDim, pGlobalWorkOffset,
6970
pGlobalWorkSize,
7071
compiledLocalWorksize.empty() ? pLocalWorkSize
7172
: compiledLocalWorksize.data(),
72-
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);
7378

7479
UR_RETURN_ON_FAILURE(createUREvent(Event, hQueue->Context, hQueue, phEvent));
7580
return UR_RESULT_SUCCESS;

unified-runtime/source/loader/ur_libapi.cpp

+3-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

unified-runtime/source/ur/ur.hpp

-2
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:

unified-runtime/source/ur_api.cpp

+3-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

unified-runtime/test/conformance/enqueue/urEnqueueKernelLaunch.cpp

-23
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)