Skip to content

Commit 1e6d671

Browse files
author
Lamzed-Short, Andrew
committed
More robust handling of out param values in piDeviceGetInfo
Moved mask location, added bitwise operations to convert CL to PI enum values, masked off relevant bits, and improved handling of out parameters.
1 parent 657da82 commit 1e6d671

File tree

2 files changed

+56
-12
lines changed

2 files changed

+56
-12
lines changed

sycl/include/sycl/detail/pi.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -554,8 +554,6 @@ constexpr pi_memory_order_capabilities PI_MEMORY_ORDER_RELEASE = 0x04;
554554
constexpr pi_memory_order_capabilities PI_MEMORY_ORDER_ACQ_REL = 0x08;
555555
constexpr pi_memory_order_capabilities PI_MEMORY_ORDER_SEQ_CST = 0x10;
556556

557-
constexpr pi_memory_order_capabilities PI_MEMORY_ORDER_BITMASK = 0x07;
558-
559557
using pi_memory_scope_capabilities = pi_bitfield;
560558
constexpr pi_memory_scope_capabilities PI_MEMORY_SCOPE_WORK_ITEM = 0x01;
561559
constexpr pi_memory_scope_capabilities PI_MEMORY_SCOPE_SUB_GROUP = 0x02;

sycl/plugins/opencl/pi_opencl.cpp

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -296,23 +296,69 @@ pi_result piDeviceGetInfo(pi_device device, pi_device_info paramName,
296296

297297
if (devVer < OCLV::V2_0) {
298298
// For OpenCL 1.2, return the minimum required values
299-
cl_int result = PI_MEMORY_ORDER_RELAXED;
300-
std::memcpy(paramValue, &result, sizeof(cl_int));
301-
return PI_SUCCESS;
299+
if (paramValue && paramValueSize < sizeof(cl_int))
300+
return static_cast<pi_result>(CL_INVALID_VALUE);
301+
if (paramValueSizeRet)
302+
*paramValueSizeRet = sizeof(cl_int);
303+
304+
if (paramValue) {
305+
cl_int capabilities = PI_MEMORY_ORDER_RELAXED;
306+
std::memcpy(paramValue, &capabilities, sizeof(cl_int));
307+
}
308+
return static_cast<pi_result>(CL_SUCCESS);
302309
} else if (devVer < OCLV::V3_0) {
303310
// For OpenCL 2.x, return all capabilities
304311
// (https://registry.khronos.org/OpenCL/specs/3.0-unified/html/OpenCL_API.html#_memory_consistency_model)
305-
cl_int result = PI_MEMORY_ORDER_RELAXED | PI_MEMORY_ORDER_ACQ_REL |
312+
if (paramValue && paramValueSize < sizeof(cl_int))
313+
return static_cast<pi_result>(CL_INVALID_VALUE);
314+
if (paramValueSizeRet)
315+
*paramValueSizeRet = sizeof(cl_int);
316+
317+
if (paramValue) {
318+
cl_int capabilities = PI_MEMORY_ORDER_RELAXED | PI_MEMORY_ORDER_ACQUIRE | PI_MEMORY_ORDER_RELEASE | PI_MEMORY_ORDER_ACQ_REL |
306319
PI_MEMORY_ORDER_SEQ_CST;
307-
std::memcpy(paramValue, &result, sizeof(cl_int));
308-
return PI_SUCCESS;
309-
} else {
320+
std::memcpy(paramValue, &capabilities, sizeof(cl_int));
321+
}
322+
return static_cast<pi_result>(CL_SUCCESS);
323+
}
324+
#ifdef CL_VERSION_3_0
325+
if (devVer >= OCLV::V3_0) {
310326
// For OpenCL >=3.0, the query should be implemented
311-
cl_int result = clGetDeviceInfo(
327+
cl_int capabilities = CL_DEVICE_ATOMIC_ORDER_RELAXED;
328+
cl_int ret_err = clGetDeviceInfo(
312329
cast<cl_device_id>(device), cast<cl_device_info>(paramName),
313-
paramValueSize, paramValue, paramValueSizeRet);
314-
return static_cast<pi_result>(result & PI_MEMORY_ORDER_BITMASK);
330+
paramValueSize, &result, paramValueSizeRet);
331+
if (ret_err != CL_SUCCESS)
332+
return cast<pi_result>(ret_err);
333+
334+
if (paramValue && paramValueSize < sizeof(cl_int))
335+
return static_cast<pi_result>(CL_INVALID_VALUE);
336+
if (paramValueSizeRet)
337+
*paramValueSizeRet = sizeof(cl_int);
338+
339+
if (paramValue) {
340+
// Mask operation to only consider atomic_memory_order* capabilities
341+
cl_int mask = CL_DEVICE_ATOMIC_ORDER_RELAXED | CL_DEVICE_ATOMIC_ORDER_ACQ_REL | CL_DEVICE_ATOMIC_ORDER_SEQ_CST;
342+
capabilities &= mask;
343+
344+
// Convert from OCL bitfield to SYCL PI bitfield
345+
// OCL could return (masked) 00000111 for all capabilities
346+
// PI would want that to be ...11111 for all capabilities as well as ACQUIRE and RELEASE
347+
// So need to bitshift and fill in result
348+
if (capabilities & CL_DEVICE_ATOMIC_ORDER_SEQ_CST) {
349+
capabilities &= ~CL_DEVICE_ATOMIC_ORDER_SEQ_CST;
350+
capabilities |= PI_MEMORY_ORDER_SEQ_CST;
351+
}
352+
if (capabilities & CL_DEVICE_ATOMIC_ORDER_ACQ_REL) {
353+
capabilities &= ~CL_DEVICE_ATOMIC_ORDER_ACQ_REL;
354+
capabilities |= (PI_MEMORY_ORDER_ACQ_REL | PI_MEMORY_ORDER_ACQUIRE | PI_MEMORY_ORDER_RELEASE);
355+
}
356+
357+
std::memcpy(paramValue, &capabilities, sizeof(cl_int));
358+
}
315359
}
360+
#endif
361+
return static_cast<pi_result>(CL_SUCCESS);
316362
}
317363
case PI_DEVICE_INFO_ATOMIC_64: {
318364
cl_int ret_err = CL_SUCCESS;

0 commit comments

Comments
 (0)