Skip to content

Commit e6ca4f9

Browse files
committed
Address code review feedback
- Renamed PI_DEVICE_INFO_HOMOGENEOUS_ARCH to PI_DEVICE_INFO_BUILD_ON_SUBDEVICE - Aligned OpenCL backend with Level Zero backend
1 parent ce299cd commit e6ca4f9

File tree

7 files changed

+29
-32
lines changed

7 files changed

+29
-32
lines changed

sycl/include/CL/sycl/detail/pi.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,8 @@ typedef enum {
305305
PI_DEVICE_INFO_GPU_EU_COUNT_PER_SUBSLICE = 0x10025,
306306
PI_DEVICE_INFO_MAX_MEM_BANDWIDTH = 0x10026,
307307
PI_DEVICE_INFO_IMAGE_SRGB = 0x10027,
308-
PI_DEVICE_INFO_HOMOGENEOUS_ARCH = 0x10028,
308+
// Return true if sub-device should do its own program build
309+
PI_DEVICE_INFO_BUILD_ON_SUBDEVICE = 0x10028,
309310
PI_DEVICE_INFO_ATOMIC_64 = 0x10110,
310311
PI_DEVICE_INFO_ATOMIC_MEMORY_ORDER_CAPABILITIES = 0x10111,
311312
PI_DEVICE_INFO_ATOMIC_MEMORY_SCOPE_CAPABILITIES = 0x11000,

sycl/plugins/cuda/pi_cuda.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1490,9 +1490,9 @@ pi_result cuda_piDeviceGetInfo(pi_device device, pi_device_info param_name,
14901490
return getInfo(param_value_size, param_value, param_value_size_ret,
14911491
PI_TRUE);
14921492
}
1493-
case PI_DEVICE_INFO_HOMOGENEOUS_ARCH: {
1493+
case PI_DEVICE_INFO_BUILD_ON_SUBDEVICE: {
14941494
return getInfo(param_value_size, param_value, param_value_size_ret,
1495-
PI_FALSE);
1495+
PI_TRUE);
14961496
}
14971497
case PI_DEVICE_INFO_COMPILER_AVAILABLE: {
14981498
return getInfo(param_value_size, param_value, param_value_size_ret,

sycl/plugins/esimd_emulator/pi_esimd_emulator.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -586,8 +586,8 @@ pi_result piDeviceGetInfo(pi_device Device, pi_device_info ParamName,
586586
return ReturnValue("");
587587
case PI_DEVICE_INFO_VERSION:
588588
return ReturnValue(CmEmuDeviceVersionString);
589-
case PI_DEVICE_INFO_HOMOGENEOUS_ARCH: // emulator doesn't support partition
590-
return ReturnValue(pi_bool{false});
589+
case PI_DEVICE_INFO_BUILD_ON_SUBDEVICE: // emulator doesn't support partition
590+
return ReturnValue(pi_bool{true});
591591
case PI_DEVICE_INFO_COMPILER_AVAILABLE:
592592
return ReturnValue(pi_bool{false});
593593
case PI_DEVICE_INFO_LINKER_AVAILABLE:

sycl/plugins/hip/pi_hip.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1409,9 +1409,9 @@ pi_result hip_piDeviceGetInfo(pi_device device, pi_device_info param_name,
14091409
return getInfo(param_value_size, param_value, param_value_size_ret,
14101410
PI_TRUE);
14111411
}
1412-
case PI_DEVICE_INFO_HOMOGENEOUS_ARCH: {
1412+
case PI_DEVICE_INFO_BUILD_ON_SUBDEVICE: {
14131413
return getInfo(param_value_size, param_value, param_value_size_ret,
1414-
PI_FALSE);
1414+
PI_TRUE);
14151415
}
14161416
case PI_DEVICE_INFO_COMPILER_AVAILABLE: {
14171417
return getInfo(param_value_size, param_value, param_value_size_ret,

sycl/plugins/level_zero/pi_level_zero.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2337,8 +2337,11 @@ pi_result piDeviceGetInfo(pi_device Device, pi_device_info ParamName,
23372337
}
23382338
case PI_DEVICE_INFO_NAME:
23392339
return ReturnValue(Device->ZeDeviceProperties->name);
2340-
case PI_DEVICE_INFO_HOMOGENEOUS_ARCH:
2341-
return ReturnValue(PI_TRUE);
2340+
// zeModuleCreate allows using root device module for sub-devices:
2341+
// > The application must only use the module for the device, or its
2342+
// > sub-devices, which was provided during creation.
2343+
case PI_DEVICE_INFO_BUILD_ON_SUBDEVICE:
2344+
return ReturnValue(PI_FALSE);
23422345
case PI_DEVICE_INFO_COMPILER_AVAILABLE:
23432346
return ReturnValue(pi_bool{1});
23442347
case PI_DEVICE_INFO_LINKER_AVAILABLE:

sycl/plugins/opencl/pi_opencl.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,10 +203,14 @@ pi_result piDeviceGetInfo(pi_device device, pi_device_info paramName,
203203
std::memcpy(paramValue, &result, sizeof(cl_bool));
204204
return PI_SUCCESS;
205205
}
206-
case PI_DEVICE_INFO_HOMOGENEOUS_ARCH: {
207-
// FIXME: conservatively return false due to lack of low-level API exposing
208-
// actual status of this property
209-
cl_bool result = false;
206+
case PI_DEVICE_INFO_BUILD_ON_SUBDEVICE: {
207+
cl_device_type devType = CL_DEVICE_TYPE_DEFAULT;
208+
cl_int res = clGetDeviceInfo(cast<cl_device_id>(device), CL_DEVICE_TYPE,
209+
sizeof(cl_device_type), &devType, nullptr);
210+
211+
// FIXME: here we assume that program built for a root GPU device can be
212+
// used on its sub-devices without re-building
213+
cl_bool result = (res == CL_SUCCESS) && (devType == CL_DEVICE_TYPE_GPU);
210214
std::memcpy(paramValue, &result, sizeof(cl_bool));
211215
return PI_SUCCESS;
212216
}

sycl/source/detail/program_manager/program_manager.cpp

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -481,36 +481,25 @@ RT::PiProgram ProgramManager::getBuiltPIProgram(
481481
if (Prg)
482482
Prg->stableSerializeSpecConstRegistry(SpecConsts);
483483

484-
// Check if root device architecture is homogeneous and we can optimize builds
485-
// for sub-devices
484+
// Check if we can optimize program builds for sub-devices by using a program
485+
// built for the root device
486486
DeviceImplPtr RootDevImpl = DeviceImpl;
487487
while (!RootDevImpl->isRootDevice()) {
488488
auto ParentDev = detail::getSyclObjImpl(
489489
RootDevImpl->get_info<info::device::parent_device>());
490+
// Sharing is allowed within a single context only
490491
if (!ContextImpl->hasDevice(ParentDev))
491492
break;
492493
RootDevImpl = ParentDev;
493494
}
494495

495-
pi_bool IsRootDeviceArchHomogeneous = PI_FALSE;
496+
pi_bool MustBuildOnSubdevice = PI_TRUE;
496497
ContextImpl->getPlugin().call<PiApiKind::piDeviceGetInfo>(
497-
RootDevImpl->getHandleRef(), PI_DEVICE_INFO_HOMOGENEOUS_ARCH,
498-
sizeof(pi_bool), &IsRootDeviceArchHomogeneous, nullptr);
499-
500-
// FIXME: the logic is modified to work around unintuitive Intel OpenCL CPU
501-
// implementation behavior. Kernels created with the program built for root
502-
// device can be re-used on sub-devices, but other combinations doesn't work
503-
// (e.g. clGetKernelWorkGroupInfo returns CL_INVALID_KERNEL if kernel was
504-
// created from the program built for sub-device and re-used either on root or
505-
// other sub-device).
506-
// To work around this case we optimize only one case: root device shares the
507-
// same context with its sub-device(s). We built for the root device and
508-
// cache the results.
509-
// The expected solution is to build for any sub-device and use root device
510-
// handle as cache key to share build results for any other sub-device or even
511-
// a root device.
498+
RootDevImpl->getHandleRef(), PI_DEVICE_INFO_BUILD_ON_SUBDEVICE,
499+
sizeof(pi_bool), &MustBuildOnSubdevice, nullptr);
500+
512501
DeviceImplPtr Dev =
513-
(IsRootDeviceArchHomogeneous == PI_TRUE) ? RootDevImpl : DeviceImpl;
502+
(MustBuildOnSubdevice == PI_TRUE) ? DeviceImpl : RootDevImpl;
514503
auto BuildF = [this, &M, &KSId, &ContextImpl, &Dev, Prg, &CompileOpts,
515504
&LinkOpts, &JITCompilationIsRequired, SpecConsts] {
516505
auto Context = createSyclObjFromImpl<context>(ContextImpl);

0 commit comments

Comments
 (0)