[SYCL] Fix platform::get_devices

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.

Signed-off-by: Larsen, Steffen <[email protected]>

Author: steffenlarsen

sycl/source/detail/allowlist.cpp

@@ -399,22 +399,25 @@ void applyAllowList(std::vector<ur_device_handle_t> &UrDevices,
         Device, UR_DEVICE_INFO_TYPE, sizeof(UrDevType), &UrDevType, nullptr);
     // TODO need mechanism to do these casts, there's a bunch of this sort of
     // thing
-    sycl::info::device_type DeviceType = info::device_type::all;
-    switch (UrDevType) {
-    default:
-    case UR_DEVICE_TYPE_ALL:
-      DeviceType = info::device_type::all;
-      break;
-    case UR_DEVICE_TYPE_GPU:
-      DeviceType = info::device_type::gpu;
-      break;
-    case UR_DEVICE_TYPE_CPU:
-      DeviceType = info::device_type::cpu;
-      break;
-    case UR_DEVICE_TYPE_FPGA:
-      DeviceType = info::device_type::accelerator;
-      break;
-    }
+    sycl::info::device_type DeviceType = [UrDevType]() {
+      switch (UrDevType) {
+      default:
+      case UR_DEVICE_TYPE_ALL:
+        return info::device_type::all;
+      case UR_DEVICE_TYPE_GPU:
+        return info::device_type::gpu;
+      case UR_DEVICE_TYPE_CPU:
+        return info::device_type::cpu;
+      case UR_DEVICE_TYPE_FPGA:
+        return info::device_type::accelerator;
+      case UR_DEVICE_TYPE_CUSTOM:
+      case UR_DEVICE_TYPE_MCA:
+      case UR_DEVICE_TYPE_VPU:
+        return info::device_type::custom;
+      case UR_DEVICE_TYPE_DEFAULT:
+        return info::device_type::automatic;
+      }
+    }();
     for (const auto &SyclDeviceType :
          getSyclDeviceTypeMap<true /*Enable 'acc'*/>()) {
       if (SyclDeviceType.second == DeviceType) {

sycl/source/detail/device_impl.hpp

@@ -596,6 +596,7 @@ class device_impl : public std::enable_shared_from_this<device_impl> {
         return device_type::accelerator;
       case UR_DEVICE_TYPE_MCA:
       case UR_DEVICE_TYPE_VPU:
+      case UR_DEVICE_TYPE_CUSTOM:
         return device_type::custom;
       default: {
         assert(false);

sycl/source/detail/platform_impl.cpp

@@ -248,24 +248,25 @@ platform_impl::filterDeviceFilter(std::vector<ur_device_handle_t> &UrDevices,
     MAdapter->call<UrApiKind::urDeviceGetInfo>(Device, UR_DEVICE_INFO_TYPE,
                                                sizeof(ur_device_type_t),
                                                &UrDevType, nullptr);
-    // Assumption here is that there is 1-to-1 mapping between UrDevType and
-    // Sycl device type for GPU, CPU, and ACC.
-    info::device_type DeviceType = info::device_type::all;
-    switch (UrDevType) {
-    default:
-    case UR_DEVICE_TYPE_ALL:
-      DeviceType = info::device_type::all;
-      break;
-    case UR_DEVICE_TYPE_GPU:
-      DeviceType = info::device_type::gpu;
-      break;
-    case UR_DEVICE_TYPE_CPU:
-      DeviceType = info::device_type::cpu;
-      break;
-    case UR_DEVICE_TYPE_FPGA:
-      DeviceType = info::device_type::accelerator;
-      break;
-    }
+    info::device_type DeviceType = [UrDevType]() {
+      switch (UrDevType) {
+      default:
+      case UR_DEVICE_TYPE_ALL:
+        return info::device_type::all;
+      case UR_DEVICE_TYPE_GPU:
+        return info::device_type::gpu;
+      case UR_DEVICE_TYPE_CPU:
+        return info::device_type::cpu;
+      case UR_DEVICE_TYPE_FPGA:
+        return info::device_type::accelerator;
+      case UR_DEVICE_TYPE_CUSTOM:
+      case UR_DEVICE_TYPE_MCA:
+      case UR_DEVICE_TYPE_VPU:
+        return info::device_type::custom;
+      case UR_DEVICE_TYPE_DEFAULT:
+        return info::device_type::automatic;
+      }
+    }();
 
     for (const FilterT &Filter : FilterList->get()) {
       backend FilterBackend = Filter.Backend.value_or(backend::all);
@@ -469,34 +470,57 @@ static std::vector<device> amendDeviceAndSubDevices(
 std::vector<device>
 platform_impl::get_devices(info::device_type DeviceType) const {
   std::vector<device> Res;
-
-  ods_target_list *OdsTargetList = SYCLConfig<ONEAPI_DEVICE_SELECTOR>::get();
+  // Host is no longer supported, so it returns an empty vector.
   if (DeviceType == info::device_type::host)
+    return std::vector<device>{};
+
+  // For custom devices, UR has additional type enums.
+  if (DeviceType == info::device_type::custom) {
+    getDevicesImplHelper(UR_DEVICE_TYPE_CUSTOM, Res);
+    getDevicesImplHelper(UR_DEVICE_TYPE_MCA, Res);
+    getDevicesImplHelper(UR_DEVICE_TYPE_VPU, Res);
+    // Some backends may return the MCA and VPU types as part of custom, so
+    // remove duplicates.
+    std::sort(Res.begin(), Res.end(),
+              [](const sycl::device &D1, const sycl::device &D2) {
+                std::hash<sycl::device> Hasher;
+                return Hasher(D1) < Hasher(D2);
+              });
+    auto NewEnd = std::unique(Res.begin(), Res.end());
+    Res.erase(NewEnd, Res.end());
     return Res;
+  }
 
-  ur_device_type_t UrDeviceType = UR_DEVICE_TYPE_ALL;
-
+  ur_device_type_t UrDeviceType = [DeviceType]() {
   switch (DeviceType) {
-  default:
   case info::device_type::all:
-    UrDeviceType = UR_DEVICE_TYPE_ALL;
-    break;
+    return UR_DEVICE_TYPE_ALL;
   case info::device_type::gpu:
-    UrDeviceType = UR_DEVICE_TYPE_GPU;
-    break;
+    return UR_DEVICE_TYPE_GPU;
   case info::device_type::cpu:
-    UrDeviceType = UR_DEVICE_TYPE_CPU;
-    break;
+    return UR_DEVICE_TYPE_CPU;
   case info::device_type::accelerator:
-    UrDeviceType = UR_DEVICE_TYPE_FPGA;
-    break;
+    return UR_DEVICE_TYPE_FPGA;
+  case info::device_type::automatic:
+    return UR_DEVICE_TYPE_DEFAULT;
+  default:
+    throw sycl::exception(sycl::make_error_code(sycl::errc::invalid),
+                          "Unknown device type.");
   }
+  }();
+  getDevicesImplHelper(UrDeviceType, Res);
+  return Res;
+}
+
+void
+platform_impl::getDevicesImplHelper(ur_device_type_t UrDeviceType,
+                                    std::vector<device> &OutVec) const {
+  size_t InitialOutVecSize = OutVec.size();
 
   uint32_t NumDevices = 0;
   MAdapter->call<UrApiKind::urDeviceGet>(MPlatform, UrDeviceType,
                                          0u, // CP info::device_type::all
                                          nullptr, &NumDevices);
-  const backend Backend = getBackend();
 
   if (NumDevices == 0) {
     // If platform doesn't have devices (even without filter)
@@ -514,7 +538,7 @@ platform_impl::get_devices(info::device_type DeviceType) const {
       std::lock_guard<std::mutex> Guard(*Adapter->getAdapterMutex());
       Adapter->adjustLastDeviceId(MPlatform);
     }
-    return Res;
+    return;
   }
 
   std::vector<ur_device_handle_t> UrDevices(NumDevices);
@@ -532,6 +556,8 @@ platform_impl::get_devices(info::device_type DeviceType) const {
   if (SYCLConfig<SYCL_DEVICE_ALLOWLIST>::get())
     applyAllowList(UrDevices, MPlatform, *MAdapter);
 
+  ods_target_list *OdsTargetList = SYCLConfig<ONEAPI_DEVICE_SELECTOR>::get();
+
   // The first step is to filter out devices that are not compatible with
   // ONEAPI_DEVICE_SELECTOR. This is also the mechanism by which top level
   // device ids are assigned.
@@ -544,7 +570,7 @@ platform_impl::get_devices(info::device_type DeviceType) const {
   // The next step is to inflate the filtered UrDevices into SYCL Device
   // objects.
   platform_impl &PlatformImpl = getOrMakePlatformImpl(MPlatform, *MAdapter);
-  std::transform(UrDevices.begin(), UrDevices.end(), std::back_inserter(Res),
+  std::transform(UrDevices.begin(), UrDevices.end(), std::back_inserter(OutVec),
                  [&PlatformImpl](const ur_device_handle_t UrDevice) -> device {
                    return detail::createSyclObjFromImpl<device>(
                        PlatformImpl.getOrMakeDeviceImpl(UrDevice));
@@ -556,15 +582,15 @@ platform_impl::get_devices(info::device_type DeviceType) const {
     MAdapter->call<UrApiKind::urDeviceRelease>(UrDev);
 
   // If we aren't using ONEAPI_DEVICE_SELECTOR, then we are done.
-  // and if there are no devices so far, there won't be any need to replace them
+  // and if there are no new devices, there won't be any need to replace them
   // with subdevices.
-  if (!OdsTargetList || Res.size() == 0)
-    return Res;
+  if (!OdsTargetList || OutVec.size() == InitialOutVecSize)
+    return;
 
   // Otherwise, our last step is to revisit the devices, possibly replacing
   // them with subdevices (which have been ignored until now)
-  return amendDeviceAndSubDevices(Backend, Res, OdsTargetList,
-                                  PlatformDeviceIndices, PlatformImpl);
+  OutVec = amendDeviceAndSubDevices(getBackend(), OutVec, OdsTargetList,
+                                    PlatformDeviceIndices, PlatformImpl);
 }
 
 bool platform_impl::has_extension(const std::string &ExtensionName) const {

sycl/source/detail/platform_impl.hpp

@@ -203,6 +203,9 @@ class platform_impl : public std::enable_shared_from_this<platform_impl> {
 private:
   device_impl *getDeviceImplHelper(ur_device_handle_t UrDevice);
 
+  void getDevicesImplHelper(ur_device_type_t UrDeviceType,
+                            std::vector<device> &OutVec) const;
+
   // Helper to get the vector of platforms supported by a given UR adapter
   static std::vector<platform> getAdapterPlatforms(adapter_impl &Adapter,
                                                    bool Supported = true);

sycl/test-e2e/Basic/platform_get_devices.cpp

@@ -0,0 +1,135 @@
+// RUN: %{build} -o %t.out
+// RUN: %{run} %t.out
+//
+// Tests platform::get_devices for each device type.
+
+#include <sycl/detail/core.hpp>
+#include <sycl/platform.hpp>
+#include <unordered_set>
+
+std::string BackendToString(sycl::backend Backend) {
+  switch (Backend) {
+  case sycl::backend::host:
+    return "host";
+  case sycl::backend::opencl:
+    return "opencl";
+  case sycl::backend::ext_oneapi_level_zero:
+    return "ext_oneapi_level_zero";
+  case sycl::backend::ext_oneapi_cuda:
+    return "ext_oneapi_cuda";
+  case sycl::backend::all:
+    return "all";
+  case sycl::backend::ext_oneapi_hip:
+    return "ext_oneapi_hip";
+  case sycl::backend::ext_oneapi_native_cpu:
+    return "ext_oneapi_native_cpu";
+  case sycl::backend::ext_oneapi_offload:
+    return "ext_oneapi_offload";
+  default:
+    return "UNKNOWN";
+  }
+}
+
+std::string DeviceTypeToString(sycl::info::device_type DevType) {
+  switch (DevType) {
+  case sycl::info::device_type::all:
+    return "device_type::all";
+  case sycl::info::device_type::cpu:
+    return "device_type::cpu";
+  case sycl::info::device_type::gpu:
+    return "device_type::gpu";
+  case sycl::info::device_type::accelerator:
+    return "device_type::accelerator";
+  case sycl::info::device_type::custom:
+    return "device_type::custom";
+  case sycl::info::device_type::automatic:
+    return "device_type::automatic";
+  case sycl::info::device_type::host:
+    return "device_type::host";
+  default:
+    return "UNKNOWN";
+  }
+}
+
+template <typename T1, typename T2>
+int Check(const T1 &LHS, const T2 &RHS, std::string TestName) {
+  if (LHS != RHS) {
+    std::cout << "Failed check " << LHS << " != " << RHS << ": " << TestName
+              << std::endl;
+    return 1;
+  }
+  return 0;
+}
+
+int CheckDeviceType(const sycl::platform &P, sycl::info::device_type DevType,
+                    std::unordered_set<sycl::device> &AllDevices) {
+  assert(DevType != sycl::info::device_type::all);
+  int Failures = 0;
+
+  std::vector<sycl::device> Devices = P.get_devices(DevType);
+
+  if (DevType == sycl::info::device_type::automatic) {
+    if (AllDevices.empty()) {
+      Failures += Check(
+          Devices.size(), 0,
+          "No devices reported for all query, but automatic returns a device.");
+    } else {
+      Failures += Check(Devices.size(), 1,
+                        "Number of devices for device_type::automatic query.");
+      if (Devices.size())
+        Failures +=
+            Check(AllDevices.count(Devices[0]), 1,
+                  "Device is in the set of all devices in the platform.");
+    }
+    return Failures;
+  }
+
+  // Count devices with the type;
+  size_t DevCount = 0;
+  for (sycl::device Device : Devices)
+    DevCount += (Device.get_info<sycl::info::device::device_type>() == DevType);
+
+  std::unordered_set<sycl::device> UniqueDevices{Devices.begin(),
+                                                 Devices.end()};
+  Check(Devices.size(), UniqueDevices.size(),
+        "Duplicate devices for " + DeviceTypeToString(DevType));
+
+  Failures +=
+      Check(Devices.size(), DevCount,
+            "Unexpected number of devices for " + DeviceTypeToString(DevType));
+
+  Failures += Check(
+      std::all_of(UniqueDevices.begin(), UniqueDevices.end(),
+                  [&](const auto &Dev) { return AllDevices.count(Dev) == 1; }),
+      true,
+      "Not all devices for " + DeviceTypeToString(DevType) +
+          " appear in the list of all devices");
+
+  return Failures;
+}
+
+int main() {
+  int Failures = 0;
+  for (sycl::platform P : sycl::platform::get_platforms()) {
+    std::cout << "Checking platform with backend "
+              << BackendToString(P.get_backend()) << std::endl;
+
+    std::vector<sycl::device> Devices = P.get_devices();
+    std::unordered_set<sycl::device> UniqueDevices{Devices.begin(),
+                                                   Devices.end()};
+
+    if (Check(Devices.size(), UniqueDevices.size(),
+              "Duplicate devices for device_type::all")) {
+      ++Failures;
+      // Don't trust this platform, so we continue.
+      continue;
+    }
+
+    for (sycl::info::device_type DevType :
+         {sycl::info::device_type::cpu, sycl::info::device_type::gpu,
+          sycl::info::device_type::accelerator, sycl::info::device_type::custom,
+          sycl::info::device_type::automatic, sycl::info::device_type::host})
+      Failures += CheckDeviceType(P, DevType, UniqueDevices);
+  }
+  return Failures;
+}