Skip to content

Commit 1e9b1b4

Browse files
kbenziekbenzie
authored
Merge pull request #805 from aarongreig/aaron/kernelSetArgIndirectionFix
Correct level of indirection used in KernelSetArgPointer calls.
2 parents a011f09 + d8500a3 commit 1e9b1b4

File tree

22 files changed

+44
-47
lines changed

22 files changed

+44
-47
lines changed

‎include/ur_api.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5023,8 +5023,8 @@ urKernelSetArgPointer(
50235023
ur_kernel_handle_t hKernel, ///< [in] handle of the kernel object
50245024
uint32_t argIndex, ///< [in] argument index in range [0, num args - 1]
50255025
const ur_kernel_arg_pointer_properties_t *pProperties, ///< [in][optional] pointer to USM pointer properties.
5026-
const void *pArgValue ///< [in][optional] USM pointer to memory location holding the argument
5027-
///< value. If null then argument value is considered null.
5026+
const void *pArgValue ///< [in][optional] Pointer obtained by USM allocation or virtual memory
5027+
///< mapping operation. If null then argument value is considered null.
50285028
);
50295029

50305030
///////////////////////////////////////////////////////////////////////////////

‎scripts/core/kernel.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,7 @@ params:
339339
desc: "[in][optional] pointer to USM pointer properties."
340340
- type: "const void*"
341341
name: pArgValue
342-
desc: "[in][optional] USM pointer to memory location holding the argument value. If null then argument value is considered null."
342+
desc: "[in][optional] Pointer obtained by USM allocation or virtual memory mapping operation. If null then argument value is considered null."
343343
returns:
344344
- $X_RESULT_ERROR_INVALID_KERNEL_ARGUMENT_INDEX
345345
- $X_RESULT_ERROR_INVALID_KERNEL_ARGUMENT_SIZE

‎source/adapters/cuda/kernel.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,8 @@ urKernelSetArgPointer(ur_kernel_handle_t hKernel, uint32_t argIndex,
287287
const ur_kernel_arg_pointer_properties_t *pProperties,
288288
const void *pArgValue) {
289289
std::ignore = pProperties;
290-
hKernel->setKernelArg(argIndex, sizeof(pArgValue), pArgValue);
290+
// setKernelArg is expecting a pointer to our argument
291+
hKernel->setKernelArg(argIndex, sizeof(pArgValue), &pArgValue);
291292
return UR_RESULT_SUCCESS;
292293
}
293294

‎source/adapters/hip/kernel.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,8 @@ urKernelGetSubGroupInfo(ur_kernel_handle_t hKernel, ur_device_handle_t hDevice,
275275
UR_APIEXPORT ur_result_t UR_APICALL urKernelSetArgPointer(
276276
ur_kernel_handle_t hKernel, uint32_t argIndex,
277277
const ur_kernel_arg_pointer_properties_t *, const void *pArgValue) {
278-
hKernel->setKernelArg(argIndex, sizeof(pArgValue), pArgValue);
278+
// setKernelArg is expecting a pointer to our argument
279+
hKernel->setKernelArg(argIndex, sizeof(pArgValue), &pArgValue);
279280
return UR_RESULT_SUCCESS;
280281
}
281282

‎source/adapters/level_zero/kernel.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1004,8 +1004,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urKernelSetArgPointer(
10041004
) {
10051005
std::ignore = Properties;
10061006

1007+
// KernelSetArgValue is expecting a pointer to the argument
10071008
UR_CALL(urKernelSetArgValue(Kernel, ArgIndex, sizeof(const void *), nullptr,
1008-
ArgValue));
1009+
&ArgValue));
10091010
return UR_RESULT_SUCCESS;
10101011
}
10111012

‎source/adapters/native_cpu/kernel.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -213,9 +213,7 @@ urKernelSetArgPointer(ur_kernel_handle_t hKernel, uint32_t argIndex,
213213
UR_ASSERT(hKernel, UR_RESULT_ERROR_INVALID_NULL_HANDLE);
214214
UR_ASSERT(pArgValue, UR_RESULT_ERROR_INVALID_NULL_POINTER);
215215

216-
auto ptrToPtr = reinterpret_cast<const intptr_t *>(pArgValue);
217-
auto derefPtr = reinterpret_cast<void *>(*ptrToPtr);
218-
hKernel->_args.push_back(derefPtr);
216+
hKernel->_args.push_back(const_cast<void *>(pArgValue));
219217

220218
return UR_RESULT_SUCCESS;
221219
}

‎source/adapters/null/ur_nullddi.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2446,8 +2446,8 @@ __urdlllocal ur_result_t UR_APICALL urKernelSetArgPointer(
24462446
const ur_kernel_arg_pointer_properties_t
24472447
*pProperties, ///< [in][optional] pointer to USM pointer properties.
24482448
const void *
2449-
pArgValue ///< [in][optional] USM pointer to memory location holding the argument
2450-
///< value. If null then argument value is considered null.
2449+
pArgValue ///< [in][optional] Pointer obtained by USM allocation or virtual memory
2450+
///< mapping operation. If null then argument value is considered null.
24512451
) try {
24522452
ur_result_t result = UR_RESULT_SUCCESS;
24532453

‎source/adapters/opencl/kernel.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -359,13 +359,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urKernelSetArgPointer(
359359
cl_ext::SetKernelArgMemPointerName, &FuncPtr));
360360

361361
if (FuncPtr) {
362-
/* OpenCL passes pointers by value not by reference. This means we need to
363-
* deref the arg to get the pointer value */
364-
auto PtrToPtr = reinterpret_cast<const intptr_t *>(pArgValue);
365-
auto DerefPtr = reinterpret_cast<void *>(*PtrToPtr);
366362
CL_RETURN_ON_FAILURE(FuncPtr(cl_adapter::cast<cl_kernel>(hKernel),
367363
cl_adapter::cast<cl_uint>(argIndex),
368-
DerefPtr));
364+
pArgValue));
369365
}
370366

371367
return UR_RESULT_SUCCESS;

‎source/loader/layers/sanitizer/asan_interceptor.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,7 @@ ur_result_t SanitizerInterceptor::prepareLaunch(
705705
char *ArgPointer = nullptr;
706706
UR_CALL(MemBuffer->getHandle(DeviceInfo->Handle, ArgPointer));
707707
ur_result_t URes = context.urDdiTable.Kernel.pfnSetArgPointer(
708-
Kernel, ArgIndex, nullptr, &ArgPointer);
708+
Kernel, ArgIndex, nullptr, ArgPointer);
709709
if (URes != UR_RESULT_SUCCESS) {
710710
context.logger.error(
711711
"Failed to set buffer {} as the {} arg to kernel {}: {}",
@@ -722,7 +722,7 @@ ur_result_t SanitizerInterceptor::prepareLaunch(
722722
(void *)LaunchInfo.Data, LaunchInfo.Data->NumLocalArgs,
723723
(void *)LaunchInfo.Data->LocalArgs);
724724
ur_result_t URes = context.urDdiTable.Kernel.pfnSetArgPointer(
725-
Kernel, ArgNums - 1, nullptr, &LaunchInfo.Data);
725+
Kernel, ArgNums - 1, nullptr, LaunchInfo.Data);
726726
if (URes != UR_RESULT_SUCCESS) {
727727
context.logger.error("Failed to set launch info: {}", URes);
728728
return URes;

‎source/loader/layers/tracing/ur_trcddi.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3167,8 +3167,8 @@ __urdlllocal ur_result_t UR_APICALL urKernelSetArgPointer(
31673167
const ur_kernel_arg_pointer_properties_t
31683168
*pProperties, ///< [in][optional] pointer to USM pointer properties.
31693169
const void *
3170-
pArgValue ///< [in][optional] USM pointer to memory location holding the argument
3171-
///< value. If null then argument value is considered null.
3170+
pArgValue ///< [in][optional] Pointer obtained by USM allocation or virtual memory
3171+
///< mapping operation. If null then argument value is considered null.
31723172
) {
31733173
auto pfnSetArgPointer = context.urDdiTable.Kernel.pfnSetArgPointer;
31743174

‎source/loader/layers/validation/ur_valddi.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3589,8 +3589,8 @@ __urdlllocal ur_result_t UR_APICALL urKernelSetArgPointer(
35893589
const ur_kernel_arg_pointer_properties_t
35903590
*pProperties, ///< [in][optional] pointer to USM pointer properties.
35913591
const void *
3592-
pArgValue ///< [in][optional] USM pointer to memory location holding the argument
3593-
///< value. If null then argument value is considered null.
3592+
pArgValue ///< [in][optional] Pointer obtained by USM allocation or virtual memory
3593+
///< mapping operation. If null then argument value is considered null.
35943594
) {
35953595
auto pfnSetArgPointer = context.urDdiTable.Kernel.pfnSetArgPointer;
35963596

‎source/loader/ur_ldrddi.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3230,8 +3230,8 @@ __urdlllocal ur_result_t UR_APICALL urKernelSetArgPointer(
32303230
const ur_kernel_arg_pointer_properties_t
32313231
*pProperties, ///< [in][optional] pointer to USM pointer properties.
32323232
const void *
3233-
pArgValue ///< [in][optional] USM pointer to memory location holding the argument
3234-
///< value. If null then argument value is considered null.
3233+
pArgValue ///< [in][optional] Pointer obtained by USM allocation or virtual memory
3234+
///< mapping operation. If null then argument value is considered null.
32353235
) {
32363236
ur_result_t result = UR_RESULT_SUCCESS;
32373237

‎source/loader/ur_libapi.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3863,8 +3863,8 @@ ur_result_t UR_APICALL urKernelSetArgPointer(
38633863
const ur_kernel_arg_pointer_properties_t
38643864
*pProperties, ///< [in][optional] pointer to USM pointer properties.
38653865
const void *
3866-
pArgValue ///< [in][optional] USM pointer to memory location holding the argument
3867-
///< value. If null then argument value is considered null.
3866+
pArgValue ///< [in][optional] Pointer obtained by USM allocation or virtual memory
3867+
///< mapping operation. If null then argument value is considered null.
38683868
) try {
38693869
auto pfnSetArgPointer = ur_lib::context->urDdiTable.Kernel.pfnSetArgPointer;
38703870
if (nullptr == pfnSetArgPointer) {

‎source/ur_api.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3284,8 +3284,8 @@ ur_result_t UR_APICALL urKernelSetArgPointer(
32843284
const ur_kernel_arg_pointer_properties_t
32853285
*pProperties, ///< [in][optional] pointer to USM pointer properties.
32863286
const void *
3287-
pArgValue ///< [in][optional] USM pointer to memory location holding the argument
3288-
///< value. If null then argument value is considered null.
3287+
pArgValue ///< [in][optional] Pointer obtained by USM allocation or virtual memory
3288+
///< mapping operation. If null then argument value is considered null.
32893289
) {
32903290
ur_result_t result = UR_RESULT_SUCCESS;
32913291
return result;

‎test/conformance/enqueue/urEnqueueKernelLaunch.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ TEST_P(urEnqueueKernelLaunchWithVirtualMemory, Success) {
364364
size_t global_size = alloc_size / sizeof(uint32_t);
365365
uint32_t fill_val = 42;
366366

367-
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, &virtual_ptr));
367+
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, virtual_ptr));
368368
ASSERT_SUCCESS(
369369
urKernelSetArgValue(kernel, 1, sizeof(fill_val), nullptr, &fill_val));
370370

@@ -516,7 +516,7 @@ TEST_P(urEnqueueKernelLaunchUSMLinkedList, Success) {
516516
}
517517

518518
// Run kernel which will iterate the list and modify the values
519-
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, &list_head));
519+
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, list_head));
520520
ASSERT_SUCCESS(urEnqueueKernelLaunch(queue, kernel, 1, &global_offset,
521521
&global_size, nullptr, 0, nullptr,
522522
nullptr));

‎test/conformance/exp_command_buffer/invalid_update.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ struct InvalidUpdateTest
2828
std::memset(shared_ptr, 0, allocation_size);
2929

3030
// Index 0 is output
31-
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, &shared_ptr));
31+
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, shared_ptr));
3232
// Index 1 is input scalar
3333
ASSERT_SUCCESS(
3434
urKernelSetArgValue(kernel, 1, sizeof(val), nullptr, &val));

‎test/conformance/exp_command_buffer/ndrange_update.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ struct NDRangeUpdateTest
2828
ASSERT_NE(shared_ptr, nullptr);
2929
std::memset(shared_ptr, 0, allocation_size);
3030

31-
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, &shared_ptr));
31+
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, shared_ptr));
3232

3333
// Add a 3 dimension kernel command to command-buffer and close
3434
// command-buffer

‎test/conformance/exp_command_buffer/usm_fill_kernel_update.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ struct USMFillCommandTest
2929
std::memset(shared_ptr, 0, allocation_size);
3030

3131
// Index 0 is output
32-
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, &shared_ptr));
32+
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, shared_ptr));
3333
// Index 1 is input scalar
3434
ASSERT_SUCCESS(
3535
urKernelSetArgValue(kernel, 1, sizeof(val), nullptr, &val));
@@ -223,7 +223,7 @@ struct USMMultipleFillCommandTest
223223
// kernel output.
224224
void *offset_ptr = (uint32_t *)shared_ptr + (k * elements);
225225
ASSERT_SUCCESS(
226-
urKernelSetArgPointer(kernel, 0, nullptr, &offset_ptr));
226+
urKernelSetArgPointer(kernel, 0, nullptr, offset_ptr));
227227

228228
// Each kernel has a unique fill value
229229
uint32_t fill_val = val + k;

‎test/conformance/exp_command_buffer/usm_saxpy_kernel_update.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,15 @@ struct USMSaxpyKernelTestBase
3636

3737
// Index 0 is output
3838
ASSERT_SUCCESS(
39-
urKernelSetArgPointer(kernel, 0, nullptr, &shared_ptrs[0]));
39+
urKernelSetArgPointer(kernel, 0, nullptr, shared_ptrs[0]));
4040
// Index 1 is A
4141
ASSERT_SUCCESS(urKernelSetArgValue(kernel, 1, sizeof(A), nullptr, &A));
4242
// Index 2 is X
4343
ASSERT_SUCCESS(
44-
urKernelSetArgPointer(kernel, 2, nullptr, &shared_ptrs[1]));
44+
urKernelSetArgPointer(kernel, 2, nullptr, shared_ptrs[1]));
4545
// Index 3 is Y
4646
ASSERT_SUCCESS(
47-
urKernelSetArgPointer(kernel, 3, nullptr, &shared_ptrs[2]));
47+
urKernelSetArgPointer(kernel, 3, nullptr, shared_ptrs[2]));
4848
}
4949

5050
void Validate(uint32_t *output, uint32_t *X, uint32_t *Y, uint32_t A,

‎test/conformance/integration/QueueEmptyStatus.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ struct QueueEmptyStatusTestWithParam : uur::IntegrationQueueTestWithParam {
3939
ArraySize * sizeof(uint32_t), 0, nullptr, &Event));
4040
ASSERT_NO_FATAL_FAILURE(submitBarrierIfNeeded(Event));
4141

42-
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, &SharedMem));
42+
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, SharedMem));
4343

4444
constexpr size_t global_offset = 0;
4545
constexpr size_t n_dimensions = 1;

‎test/conformance/integration/QueueUSM.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,8 @@ TEST_P(QueueUSMTestWithParam, QueueUSMTest) {
8888

8989
for (uint32_t i = 0; i < NumIterations; ++i) {
9090
/* Copy from DeviceMem2 to DeviceMem1 and multiply by 2 */
91-
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, &DeviceMem1));
92-
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 1, nullptr, &DeviceMem2));
91+
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, DeviceMem1));
92+
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 1, nullptr, DeviceMem2));
9393

9494
ASSERT_SUCCESS(urEnqueueKernelLaunch(Queue, kernel, NDimensions,
9595
&GlobalOffset, &ArraySize, nullptr,
@@ -99,8 +99,8 @@ TEST_P(QueueUSMTestWithParam, QueueUSMTest) {
9999
CurValueMem2 = CurValueMem1 * 2;
100100

101101
/* Copy from DeviceMem1 to DeviceMem2 and multiply by 2 */
102-
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, &DeviceMem2));
103-
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 1, nullptr, &DeviceMem1));
102+
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, DeviceMem2));
103+
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 1, nullptr, DeviceMem1));
104104

105105
ASSERT_SUCCESS(urEnqueueKernelLaunch(Queue, kernel, NDimensions,
106106
&GlobalOffset, &ArraySize, nullptr,

‎test/conformance/kernel/urKernelSetArgPointer.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ TEST_P(urKernelSetArgPointerTest, SuccessHost) {
4242
&allocation));
4343
ASSERT_NE(allocation, nullptr);
4444

45-
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, &allocation));
45+
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, allocation));
4646
ASSERT_SUCCESS(
4747
urKernelSetArgValue(kernel, 1, sizeof(data), nullptr, &data));
4848
Launch1DRange(array_size);
@@ -60,7 +60,7 @@ TEST_P(urKernelSetArgPointerTest, SuccessDevice) {
6060
allocation_size, &allocation));
6161
ASSERT_NE(allocation, nullptr);
6262

63-
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, &allocation));
63+
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, allocation));
6464
ASSERT_SUCCESS(
6565
urKernelSetArgValue(kernel, 1, sizeof(data), nullptr, &data));
6666
Launch1DRange(array_size);
@@ -87,7 +87,7 @@ TEST_P(urKernelSetArgPointerTest, SuccessShared) {
8787
allocation_size, &allocation));
8888
ASSERT_NE(allocation, nullptr);
8989

90-
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, &allocation));
90+
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, allocation));
9191
ASSERT_SUCCESS(
9292
urKernelSetArgValue(kernel, 1, sizeof(data), nullptr, &data));
9393
Launch1DRange(array_size);
@@ -138,7 +138,7 @@ UUR_INSTANTIATE_KERNEL_TEST_SUITE_P(urKernelSetArgPointerNegativeTest);
138138

139139
TEST_P(urKernelSetArgPointerNegativeTest, InvalidNullHandleKernel) {
140140
ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_NULL_HANDLE,
141-
urKernelSetArgPointer(nullptr, 0, nullptr, &allocation));
141+
urKernelSetArgPointer(nullptr, 0, nullptr, allocation));
142142
}
143143

144144
TEST_P(urKernelSetArgPointerNegativeTest, InvalidKernelArgumentIndex) {
@@ -149,5 +149,5 @@ TEST_P(urKernelSetArgPointerNegativeTest, InvalidKernelArgumentIndex) {
149149

150150
ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_KERNEL_ARGUMENT_INDEX,
151151
urKernelSetArgPointer(kernel, num_kernel_args + 1, nullptr,
152-
&allocation));
152+
allocation));
153153
}

0 commit comments

Comments
 (0)