Skip to content

Commit ea192fd

Browse files
steffenlarsensteffenlarsen
authored
[SYCL][UR][OpenCL][L0] Fix platform::get_devices (#19810)
In relation to the clarification in KhronosGroup/SYCL-Docs#861, the implementation of `platform::get_devices` does not work correctly for `custom` and `automatic`. This commit fixes this by ensuring these return the devices with custom type and the default device respectively. Additionally, it adds the `UR_DEVICE_TYPE_CUSTOM` to UR and adds proper handling for `UR_DEVICE_TYPE_DEFAULT` in the UR OpenCL adapter. --------- Signed-off-by: Larsen, Steffen <[email protected]>
1 parent 421bfc1 commit ea192fd

File tree

17 files changed

+250
-92
lines changed

17 files changed

+250
-92
lines changed

‎sycl/source/detail/allowlist.cpp‎

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -399,22 +399,7 @@ void applyAllowList(std::vector<ur_device_handle_t> &UrDevices,
399399
Device, UR_DEVICE_INFO_TYPE, sizeof(UrDevType), &UrDevType, nullptr);
400400
// TODO need mechanism to do these casts, there's a bunch of this sort of
401401
// thing
402-
sycl::info::device_type DeviceType = info::device_type::all;
403-
switch (UrDevType) {
404-
default:
405-
case UR_DEVICE_TYPE_ALL:
406-
DeviceType = info::device_type::all;
407-
break;
408-
case UR_DEVICE_TYPE_GPU:
409-
DeviceType = info::device_type::gpu;
410-
break;
411-
case UR_DEVICE_TYPE_CPU:
412-
DeviceType = info::device_type::cpu;
413-
break;
414-
case UR_DEVICE_TYPE_FPGA:
415-
DeviceType = info::device_type::accelerator;
416-
break;
417-
}
402+
sycl::info::device_type DeviceType = detail::ConvertDeviceType(UrDevType);
418403
for (const auto &SyclDeviceType :
419404
getSyclDeviceTypeMap<true /*Enable 'acc'*/>()) {
420405
if (SyclDeviceType.second == DeviceType) {

‎sycl/source/detail/device_impl.hpp‎

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,29 @@ ConvertAffinityDomain(const ur_device_affinity_domain_flags_t Domain) {
6464
}
6565
}
6666

67+
inline info::device_type ConvertDeviceType(ur_device_type_t UrDevType) {
68+
switch (UrDevType) {
69+
case UR_DEVICE_TYPE_DEFAULT:
70+
return info::device_type::automatic;
71+
case UR_DEVICE_TYPE_ALL:
72+
return info::device_type::all;
73+
case UR_DEVICE_TYPE_GPU:
74+
return info::device_type::gpu;
75+
case UR_DEVICE_TYPE_CPU:
76+
return info::device_type::cpu;
77+
case UR_DEVICE_TYPE_FPGA:
78+
return info::device_type::accelerator;
79+
case UR_DEVICE_TYPE_MCA:
80+
case UR_DEVICE_TYPE_VPU:
81+
case UR_DEVICE_TYPE_CUSTOM:
82+
return info::device_type::custom;
83+
default:
84+
assert(false);
85+
// FIXME: what is that???
86+
return info::device_type::custom;
87+
}
88+
}
89+
6790
// Note that UR's enums have weird *_FORCE_UINT32 values, we ignore them in the
6891
// callers. But we also can't write a fully-covered switch without mentioning it
6992
// there, which wouldn't make any sense. As such, ensure that "real" values
@@ -582,27 +605,7 @@ class device_impl : public std::enable_shared_from_this<device_impl> {
582605
// device_traits.def
583606

584607
CASE(info::device::device_type) {
585-
using device_type = info::device_type;
586-
switch (get_info_impl<UR_DEVICE_INFO_TYPE>()) {
587-
case UR_DEVICE_TYPE_DEFAULT:
588-
return device_type::automatic;
589-
case UR_DEVICE_TYPE_ALL:
590-
return device_type::all;
591-
case UR_DEVICE_TYPE_GPU:
592-
return device_type::gpu;
593-
case UR_DEVICE_TYPE_CPU:
594-
return device_type::cpu;
595-
case UR_DEVICE_TYPE_FPGA:
596-
return device_type::accelerator;
597-
case UR_DEVICE_TYPE_MCA:
598-
case UR_DEVICE_TYPE_VPU:
599-
return device_type::custom;
600-
default: {
601-
assert(false);
602-
// FIXME: what is that???
603-
return device_type::custom;
604-
}
605-
}
608+
return detail::ConvertDeviceType(get_info_impl<UR_DEVICE_INFO_TYPE>());
606609
}
607610

608611
CASE(info::device::max_work_item_sizes<3>) {

‎sycl/source/detail/platform_impl.cpp‎

Lines changed: 52 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -248,24 +248,7 @@ platform_impl::filterDeviceFilter(std::vector<ur_device_handle_t> &UrDevices,
248248
MAdapter->call<UrApiKind::urDeviceGetInfo>(Device, UR_DEVICE_INFO_TYPE,
249249
sizeof(ur_device_type_t),
250250
&UrDevType, nullptr);
251-
// Assumption here is that there is 1-to-1 mapping between UrDevType and
252-
// Sycl device type for GPU, CPU, and ACC.
253-
info::device_type DeviceType = info::device_type::all;
254-
switch (UrDevType) {
255-
default:
256-
case UR_DEVICE_TYPE_ALL:
257-
DeviceType = info::device_type::all;
258-
break;
259-
case UR_DEVICE_TYPE_GPU:
260-
DeviceType = info::device_type::gpu;
261-
break;
262-
case UR_DEVICE_TYPE_CPU:
263-
DeviceType = info::device_type::cpu;
264-
break;
265-
case UR_DEVICE_TYPE_FPGA:
266-
DeviceType = info::device_type::accelerator;
267-
break;
268-
}
251+
info::device_type DeviceType = detail::ConvertDeviceType(UrDevType);
269252

270253
for (const FilterT &Filter : FilterList->get()) {
271254
backend FilterBackend = Filter.Backend.value_or(backend::all);
@@ -469,34 +452,56 @@ static std::vector<device> amendDeviceAndSubDevices(
469452
std::vector<device>
470453
platform_impl::get_devices(info::device_type DeviceType) const {
471454
std::vector<device> Res;
472-
473-
ods_target_list *OdsTargetList = SYCLConfig<ONEAPI_DEVICE_SELECTOR>::get();
455+
// Host is no longer supported, so it returns an empty vector.
474456
if (DeviceType == info::device_type::host)
457+
return std::vector<device>{};
458+
459+
// For custom devices, UR has additional type enums.
460+
if (DeviceType == info::device_type::custom) {
461+
getDevicesImplHelper(UR_DEVICE_TYPE_CUSTOM, Res);
462+
getDevicesImplHelper(UR_DEVICE_TYPE_MCA, Res);
463+
getDevicesImplHelper(UR_DEVICE_TYPE_VPU, Res);
464+
// Some backends may return the MCA and VPU types as part of custom, so
465+
// remove duplicates.
466+
std::sort(Res.begin(), Res.end(),
467+
[](const sycl::device &D1, const sycl::device &D2) {
468+
std::hash<sycl::device> Hasher;
469+
return Hasher(D1) < Hasher(D2);
470+
});
471+
auto NewEnd = std::unique(Res.begin(), Res.end());
472+
Res.erase(NewEnd, Res.end());
475473
return Res;
476-
477-
ur_device_type_t UrDeviceType = UR_DEVICE_TYPE_ALL;
478-
479-
switch (DeviceType) {
480-
default:
481-
case info::device_type::all:
482-
UrDeviceType = UR_DEVICE_TYPE_ALL;
483-
break;
484-
case info::device_type::gpu:
485-
UrDeviceType = UR_DEVICE_TYPE_GPU;
486-
break;
487-
case info::device_type::cpu:
488-
UrDeviceType = UR_DEVICE_TYPE_CPU;
489-
break;
490-
case info::device_type::accelerator:
491-
UrDeviceType = UR_DEVICE_TYPE_FPGA;
492-
break;
493474
}
494475

476+
ur_device_type_t UrDeviceType = [DeviceType]() {
477+
switch (DeviceType) {
478+
case info::device_type::all:
479+
return UR_DEVICE_TYPE_ALL;
480+
case info::device_type::gpu:
481+
return UR_DEVICE_TYPE_GPU;
482+
case info::device_type::cpu:
483+
return UR_DEVICE_TYPE_CPU;
484+
case info::device_type::accelerator:
485+
return UR_DEVICE_TYPE_FPGA;
486+
case info::device_type::automatic:
487+
return UR_DEVICE_TYPE_DEFAULT;
488+
default:
489+
throw sycl::exception(sycl::make_error_code(sycl::errc::invalid),
490+
"Unknown device type.");
491+
}
492+
}();
493+
getDevicesImplHelper(UrDeviceType, Res);
494+
return Res;
495+
}
496+
497+
void platform_impl::getDevicesImplHelper(ur_device_type_t UrDeviceType,
498+
std::vector<device> &OutVec) const {
499+
size_t InitialOutVecSize = OutVec.size();
500+
495501
uint32_t NumDevices = 0;
496502
MAdapter->call<UrApiKind::urDeviceGet>(MPlatform, UrDeviceType,
497503
0u, // CP info::device_type::all
498504
nullptr, &NumDevices);
499-
const backend Backend = getBackend();
500505

501506
if (NumDevices == 0) {
502507
// If platform doesn't have devices (even without filter)
@@ -514,7 +519,7 @@ platform_impl::get_devices(info::device_type DeviceType) const {
514519
std::lock_guard<std::mutex> Guard(*Adapter->getAdapterMutex());
515520
Adapter->adjustLastDeviceId(MPlatform);
516521
}
517-
return Res;
522+
return;
518523
}
519524

520525
std::vector<ur_device_handle_t> UrDevices(NumDevices);
@@ -532,6 +537,8 @@ platform_impl::get_devices(info::device_type DeviceType) const {
532537
if (SYCLConfig<SYCL_DEVICE_ALLOWLIST>::get())
533538
applyAllowList(UrDevices, MPlatform, *MAdapter);
534539

540+
ods_target_list *OdsTargetList = SYCLConfig<ONEAPI_DEVICE_SELECTOR>::get();
541+
535542
// The first step is to filter out devices that are not compatible with
536543
// ONEAPI_DEVICE_SELECTOR. This is also the mechanism by which top level
537544
// device ids are assigned.
@@ -544,7 +551,7 @@ platform_impl::get_devices(info::device_type DeviceType) const {
544551
// The next step is to inflate the filtered UrDevices into SYCL Device
545552
// objects.
546553
platform_impl &PlatformImpl = getOrMakePlatformImpl(MPlatform, *MAdapter);
547-
std::transform(UrDevices.begin(), UrDevices.end(), std::back_inserter(Res),
554+
std::transform(UrDevices.begin(), UrDevices.end(), std::back_inserter(OutVec),
548555
[&PlatformImpl](const ur_device_handle_t UrDevice) -> device {
549556
return detail::createSyclObjFromImpl<device>(
550557
PlatformImpl.getOrMakeDeviceImpl(UrDevice));
@@ -556,15 +563,15 @@ platform_impl::get_devices(info::device_type DeviceType) const {
556563
MAdapter->call<UrApiKind::urDeviceRelease>(UrDev);
557564

558565
// If we aren't using ONEAPI_DEVICE_SELECTOR, then we are done.
559-
// and if there are no devices so far, there won't be any need to replace them
566+
// and if there are no new devices, there won't be any need to replace them
560567
// with subdevices.
561-
if (!OdsTargetList || Res.size() == 0)
562-
return Res;
568+
if (!OdsTargetList || OutVec.size() == InitialOutVecSize)
569+
return;
563570

564571
// Otherwise, our last step is to revisit the devices, possibly replacing
565572
// them with subdevices (which have been ignored until now)
566-
return amendDeviceAndSubDevices(Backend, Res, OdsTargetList,
567-
PlatformDeviceIndices, PlatformImpl);
573+
OutVec = amendDeviceAndSubDevices(getBackend(), OutVec, OdsTargetList,
574+
PlatformDeviceIndices, PlatformImpl);
568575
}
569576

570577
bool platform_impl::has_extension(const std::string &ExtensionName) const {

‎sycl/source/detail/platform_impl.hpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,9 @@ class platform_impl : public std::enable_shared_from_this<platform_impl> {
203203
private:
204204
device_impl *getDeviceImplHelper(ur_device_handle_t UrDevice);
205205

206+
void getDevicesImplHelper(ur_device_type_t UrDeviceType,
207+
std::vector<device> &OutVec) const;
208+
206209
// Helper to get the vector of platforms supported by a given UR adapter
207210
static std::vector<platform> getAdapterPlatforms(adapter_impl &Adapter,
208211
bool Supported = true);

‎sycl/test-e2e/Basic/platform_get_devices.cpp‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
// RUN: %{build} -Wno-deprecated-declarations -o %t.out
2+
// RUN: %{run} %t.out
3+
//
4+
// Tests platform::get_devices for each device type.
5+
6+
#include <sycl/detail/core.hpp>
7+
#include <sycl/platform.hpp>
8+
#include <unordered_set>
9+
10+
std::string BackendToString(sycl::backend Backend) {
11+
switch (Backend) {
12+
case sycl::backend::host:
13+
return "host";
14+
case sycl::backend::opencl:
15+
return "opencl";
16+
case sycl::backend::ext_oneapi_level_zero:
17+
return "ext_oneapi_level_zero";
18+
case sycl::backend::ext_oneapi_cuda:
19+
return "ext_oneapi_cuda";
20+
case sycl::backend::all:
21+
return "all";
22+
case sycl::backend::ext_oneapi_hip:
23+
return "ext_oneapi_hip";
24+
case sycl::backend::ext_oneapi_native_cpu:
25+
return "ext_oneapi_native_cpu";
26+
case sycl::backend::ext_oneapi_offload:
27+
return "ext_oneapi_offload";
28+
default:
29+
return "UNKNOWN";
30+
}
31+
}
32+
33+
std::string DeviceTypeToString(sycl::info::device_type DevType) {
34+
switch (DevType) {
35+
case sycl::info::device_type::all:
36+
return "device_type::all";
37+
case sycl::info::device_type::cpu:
38+
return "device_type::cpu";
39+
case sycl::info::device_type::gpu:
40+
return "device_type::gpu";
41+
case sycl::info::device_type::accelerator:
42+
return "device_type::accelerator";
43+
case sycl::info::device_type::custom:
44+
return "device_type::custom";
45+
case sycl::info::device_type::automatic:
46+
return "device_type::automatic";
47+
case sycl::info::device_type::host:
48+
return "device_type::host";
49+
default:
50+
return "UNKNOWN";
51+
}
52+
}
53+
54+
template <typename T1, typename T2>
55+
int Check(const T1 &LHS, const T2 &RHS, std::string TestName) {
56+
if (LHS != RHS) {
57+
std::cout << "Failed check " << LHS << " != " << RHS << ": " << TestName
58+
<< std::endl;
59+
return 1;
60+
}
61+
return 0;
62+
}
63+
64+
int CheckDeviceType(const sycl::platform &P, sycl::info::device_type DevType,
65+
std::unordered_set<sycl::device> &AllDevices) {
66+
assert(DevType != sycl::info::device_type::all);
67+
int Failures = 0;
68+
69+
std::vector<sycl::device> Devices = P.get_devices(DevType);
70+
71+
if (DevType == sycl::info::device_type::automatic) {
72+
if (AllDevices.empty()) {
73+
Failures += Check(
74+
Devices.size(), 0,
75+
"No devices reported for all query, but automatic returns a device.");
76+
} else {
77+
Failures += Check(Devices.size(), 1,
78+
"Number of devices for device_type::automatic query.");
79+
if (Devices.size())
80+
Failures +=
81+
Check(AllDevices.count(Devices[0]), 1,
82+
"Device is in the set of all devices in the platform.");
83+
}
84+
return Failures;
85+
}
86+
87+
// Count devices with the type;
88+
size_t DevCount = 0;
89+
for (sycl::device Device : Devices)
90+
DevCount += (Device.get_info<sycl::info::device::device_type>() == DevType);
91+
92+
std::unordered_set<sycl::device> UniqueDevices{Devices.begin(),
93+
Devices.end()};
94+
Check(Devices.size(), UniqueDevices.size(),
95+
"Duplicate devices for " + DeviceTypeToString(DevType));
96+
97+
Failures +=
98+
Check(Devices.size(), DevCount,
99+
"Unexpected number of devices for " + DeviceTypeToString(DevType));
100+
101+
Failures += Check(
102+
std::all_of(UniqueDevices.begin(), UniqueDevices.end(),
103+
[&](const auto &Dev) { return AllDevices.count(Dev) == 1; }),
104+
true,
105+
"Not all devices for " + DeviceTypeToString(DevType) +
106+
" appear in the list of all devices");
107+
108+
return Failures;
109+
}
110+
111+
int main() {
112+
int Failures = 0;
113+
for (sycl::platform P : sycl::platform::get_platforms()) {
114+
std::cout << "Checking platform with backend "
115+
<< BackendToString(P.get_backend()) << std::endl;
116+
117+
std::vector<sycl::device> Devices = P.get_devices();
118+
std::unordered_set<sycl::device> UniqueDevices{Devices.begin(),
119+
Devices.end()};
120+
121+
if (Check(Devices.size(), UniqueDevices.size(),
122+
"Duplicate devices for device_type::all")) {
123+
++Failures;
124+
// Don't trust this platform, so we continue.
125+
continue;
126+
}
127+
128+
for (sycl::info::device_type DevType :
129+
{sycl::info::device_type::cpu, sycl::info::device_type::gpu,
130+
sycl::info::device_type::accelerator, sycl::info::device_type::custom,
131+
sycl::info::device_type::automatic, sycl::info::device_type::host})
132+
Failures += CheckDeviceType(P, DevType, UniqueDevices);
133+
}
134+
return Failures;
135+
}

0 commit comments

Comments
 (0)