[SYCL] Stop deriving ext::oneapi::filter_selector from device_selector

sycl/include/sycl/device.hpp

@@ -48,9 +48,6 @@ auto getDeviceComparisonLambda();
 } // namespace detail
 
 namespace ext::oneapi {
-// Forward declaration
-class filter_selector;
-
 enum class peer_access {
   access_supported = 0x0,
   atomics_supported = 0x1,

sycl/include/sycl/device_selector.hpp

@@ -25,9 +25,11 @@ inline namespace _V1 {
 class device;
 class context;
 
+#ifndef __INTEL_PREVIEW_BREAKING_CHANGES
 namespace ext::oneapi {
 class filter_selector;
 } // namespace ext::oneapi
+#endif // __INTEL_PREVIEW_BREAKING_CHANGES
 
 /// The SYCL 1.2.1 device_selector class provides ability to choose the
 /// best SYCL device based on heuristics specified by the user.
@@ -120,14 +122,17 @@ void fill_aspect_vector(std::vector<aspect> &V, FirstT F, OtherTs... O) {
   fill_aspect_vector(V, O...);
 }
 
-// Enable if DeviceSelector callable has matching signature, but
-// exclude if descended from filter_selector which is not purely callable or
-// if descended from it is descended from SYCL 1.2.1 device_selector.
-// See [FilterSelector not Callable] in device_selector.cpp
+// Enable if DeviceSelector callable has matching signature, but exclude if it
+// is descended from the SYCL 1.2.1 device_selector, which has its own
+// (deprecated) overloads. ext::oneapi::filter_selector is only a plain callable
+// selector in the preview mode, elsewhere it still derives from
+// device_selector, so keep excluding it explicitly.
 template <typename DeviceSelector>
 using EnableIfSYCL2020DeviceSelectorInvocable = std::enable_if_t<
     std::is_invocable_r_v<int, DeviceSelector &, const device &> &&
+#ifndef __INTEL_PREVIEW_BREAKING_CHANGES
     !std::is_base_of_v<ext::oneapi::filter_selector, DeviceSelector> &&
+#endif // __INTEL_PREVIEW_BREAKING_CHANGES
     !std::is_base_of_v<device_selector, DeviceSelector>>;
 
 __SYCL_EXPORT device

sycl/include/sycl/ext/oneapi/filter_selector.hpp

@@ -8,21 +8,21 @@
 
 #pragma once
 
-#include <sycl/detail/export.hpp>   // for __SYCL_EXPORT
-#include <sycl/device.hpp>          // for device
+#include <sycl/detail/defines_elementary.hpp> // for __SYCL2020_DEPRECATED
+#include <sycl/detail/export.hpp>             // for __SYCL_EXPORT
+#include <sycl/detail/string_view.hpp>        // for string_view
+#ifndef __INTEL_PREVIEW_BREAKING_CHANGES
 #include <sycl/device_selector.hpp> // for device_selector
+#endif                              // __INTEL_PREVIEW_BREAKING_CHANGES
 
 #include <memory> // for shared_ptr
 #include <string> // for string
 
-// 4.6.1 Device selection class
-
 namespace sycl {
 inline namespace _V1 {
 
 // Forward declarations
 class device;
-class device_selector;
 #ifdef __SYCL_INTERNAL_API
 namespace ONEAPI {
 class filter_selector;
@@ -34,13 +34,35 @@ namespace detail {
 class filter_selector_impl;
 } // namespace detail
 
-class __SYCL_EXPORT filter_selector : public device_selector {
+/// Selects a device matching one or more filters of the form
+/// `Backend:DeviceType:RelativeDeviceNumber`, see
+/// sycl_ext_oneapi_filter_selector.
+///
+/// This is a SYCL 2020 callable device selector: it can be passed to the
+/// `device`, `platform` and `queue` constructors, and it may be invoked
+/// directly as many times as needed. The set of devices matching the filters is
+/// determined when the selector is constructed.
+class __SYCL_EXPORT filter_selector
+#ifndef __INTEL_PREVIEW_BREAKING_CHANGES
+    // Nothing in this class needs the deprecated SYCL 1.2.1 device_selector.
+    // The base class is only kept to preserve the ABI of the non-preview
+    // library.
+    : public device_selector
+#endif // __INTEL_PREVIEW_BREAKING_CHANGES
+{
 public:
   filter_selector(const std::string &filter)
       : filter_selector(sycl::detail::string_view{filter}) {}
+#ifdef __INTEL_PREVIEW_BREAKING_CHANGES
+  int operator()(const device &dev) const;
+#else
   int operator()(const device &dev) const override;
+  __SYCL_DEPRECATED("The selector keeps no state between the invocations of "
+                    "operator(), so there is nothing to reset.")
   void reset() const;
+  __SYCL_DEPRECATED("Construct a sycl::device from the selector instead.")
   device select_device() const override;
+#endif // __INTEL_PREVIEW_BREAKING_CHANGES
 #ifdef __SYCL_INTERNAL_API
   friend class sycl::ONEAPI::filter_selector;
 #endif
@@ -58,9 +80,13 @@ class __SYCL_EXPORT filter_selector : public ext::oneapi::filter_selector {
 public:
   filter_selector(const std::string &filter)
       : filter_selector(sycl::detail::string_view{filter}) {}
+#ifdef __INTEL_PREVIEW_BREAKING_CHANGES
+  int operator()(const device &dev) const;
+#else
   int operator()(const device &dev) const override;
   void reset() const;
   device select_device() const override;
+#endif // __INTEL_PREVIEW_BREAKING_CHANGES
 
 private:
   filter_selector(sycl::detail::string_view filter);

sycl/include/sycl/platform.hpp

@@ -60,10 +60,6 @@ class platform_impl;
 void __SYCL_EXPORT enable_ext_oneapi_default_context(bool Val);
 
 } // namespace detail
-namespace ext::oneapi {
-// Forward declaration
-class filter_selector;
-} // namespace ext::oneapi
 
 /// Encapsulates a SYCL platform on which kernels may be executed.
 ///

sycl/source/detail/filter_selector_impl.cpp

@@ -13,6 +13,7 @@
 #include <sycl/device_selector.hpp>
 #include <sycl/exception.hpp>
 
+#include <algorithm>
 #include <cctype>
 #include <regex>
 #include <string>
@@ -87,82 +88,64 @@ filter create_filter(const std::string &Input) {
   return Result;
 }
 
-filter_selector_impl::filter_selector_impl(const std::string &Input)
-    : mFilters(), mNumDevicesSeen(0), mMatchFound(false) {
-  std::vector<std::string> Filters = detail::tokenize(Input, ",");
-  mNumTotalDevices = device::get_devices().size();
-
-  for (const std::string &Filter : Filters) {
-    detail::filter F = detail::create_filter(Filter);
-    mFilters.push_back(std::move(F));
-  }
-}
-
-int filter_selector_impl::operator()(const device &Dev) const {
-  int Score = REJECT_DEVICE_SCORE;
-
-  for (auto &Filter : mFilters) {
-    bool BackendOK = true;
-    bool DeviceTypeOK = true;
-    bool DeviceNumOK = true;
-
-    if (Filter.Backend) {
-      backend BE = sycl::detail::getSyclObjImpl(Dev)->getBackend();
-      // Backend is okay if the filter BE is set 'all'.
-      if (Filter.Backend.value() == backend::all)
-        BackendOK = true;
-      else
-        BackendOK = (BE == Filter.Backend.value());
-    }
-    if (Filter.DeviceType) {
-      sycl::info::device_type DT =
-          Dev.get_info<sycl::info::device::device_type>();
-      // DeviceType is okay if the filter is set 'all'.
-      if (Filter.DeviceType == sycl::info::device_type::all)
-        DeviceTypeOK = true;
-      else
-        DeviceTypeOK = (DT == Filter.DeviceType);
-    }
-    if (Filter.DeviceNum) {
-      // Only check device number if we're good on the previous matches
-      if (BackendOK && DeviceTypeOK) {
-        // Do we match?
-        DeviceNumOK = (Filter.MatchesSeen == Filter.DeviceNum.value());
-        // Safe to increment matches even if we find it
-        Filter.MatchesSeen++;
+filter_selector_impl::filter_selector_impl(const std::string &Input) {
+  std::vector<filter> Filters;
+  for (const std::string &Filter : detail::tokenize(Input, ","))
+    Filters.push_back(detail::create_filter(Filter));
+
+  // Matching the filters requires state to be kept between the devices (to
+  // track the relative device number), so do it once here instead of doing it
+  // in operator().
+  for (const device &Dev : device::get_devices()) {
+    for (filter &Filter : Filters) {
+      bool BackendOK = true;
+      bool DeviceTypeOK = true;
+      bool DeviceNumOK = true;
+
+      if (Filter.Backend) {
+        // Backend is okay if the filter BE is set 'all'.
+        BackendOK = Filter.Backend.value() == backend::all ||
+                    sycl::detail::getSyclObjImpl(Dev)->getBackend() ==
+                        Filter.Backend.value();
+      }
+      if (Filter.DeviceType) {
+        // DeviceType is okay if the filter is set 'all'.
+        DeviceTypeOK =
+            Filter.DeviceType.value() == sycl::info::device_type::all ||
+            Dev.get_info<sycl::info::device::device_type>() ==
+                Filter.DeviceType.value();
+      }
+      if (Filter.DeviceNum) {
+        // Only check device number if we're good on the previous matches
+        if (BackendOK && DeviceTypeOK) {
+          // Do we match?
+          DeviceNumOK = (Filter.MatchesSeen == Filter.DeviceNum.value());
+          // Safe to increment matches even if we find it
+          Filter.MatchesSeen++;
+        }
+      }
+      if (BackendOK && DeviceTypeOK && DeviceNumOK) {
+        mMatchingDevices.push_back(Dev);
+        break;
       }
-    }
-    if (BackendOK && DeviceTypeOK && DeviceNumOK) {
-      Score = default_selector_v(Dev);
-      mMatchFound = true;
-      break;
     }
   }
+}
 
-  mNumDevicesSeen++;
-  if ((mNumDevicesSeen == mNumTotalDevices) && !mMatchFound) {
+int filter_selector_impl::operator()(const device &Dev) const {
+  if (mMatchingDevices.empty())
     throw exception(
         make_error_code(errc::runtime),
         "Could not find a device that matches the specified filter(s)!");
-  }
 
-  return Score;
-}
+  if (std::find(mMatchingDevices.begin(), mMatchingDevices.end(), Dev) ==
+      mMatchingDevices.end())
+    return REJECT_DEVICE_SCORE;
 
-void filter_selector_impl::reset() const {
-  // This is a bit of an abuse of "const" method...
-  // Reset state if you want to reuse this selector.
-  for (auto &Filter : mFilters) {
-    Filter.MatchesSeen = 0;
-  }
-  mMatchFound = false;
-  mNumDevicesSeen = 0;
+  // Let the default selector rank the devices that passed the filters.
+  return default_selector_v(Dev);
 }
 
 } // namespace ext::oneapi::detail
-
-namespace __SYCL2020_DEPRECATED("use 'ext::oneapi' instead") ONEAPI {
-using namespace ext::oneapi;
-}
 } // namespace _V1
 } // namespace sycl

sycl/source/detail/filter_selector_impl.hpp

@@ -8,35 +8,33 @@
 
 #pragma once
 
+#include <sycl/detail/defines_elementary.hpp>
 #include <sycl/detail/device_filter.hpp>
-#include <sycl/device_selector.hpp>
+#include <sycl/device.hpp>
 
+#include <string>
 #include <vector>
 
 namespace sycl {
 inline namespace _V1 {
 
-// Forward declarations
-class device;
-
 namespace ext {
 namespace oneapi {
 namespace detail {
 
 using filter = sycl::detail::ods_target;
 
+/// The set of devices matching the filter string is computed once, when the
+/// selector is created. That keeps operator() a pure function, so that the
+/// selector can be used as a SYCL 2020 callable device selector.
 class filter_selector_impl {
 public:
   filter_selector_impl(const std::string &filter);
   int operator()(const device &dev) const;
-  void reset() const;
 
 private:
   static constexpr int REJECT_DEVICE_SCORE = -1;
-  mutable std::vector<filter> mFilters;
-  mutable int mNumDevicesSeen;
-  int mNumTotalDevices;
-  mutable bool mMatchFound;
+  std::vector<device> mMatchingDevices;
 };
 } // namespace detail
 } // namespace oneapi

sycl/source/detail/global_handler.cpp

@@ -190,11 +190,6 @@ std::mutex &GlobalHandler::getPlatformMapMutex() {
   return PlatformMapMutex;
 }
 
-std::mutex &GlobalHandler::getFilterMutex() {
-  static std::mutex &FilterMutex = getOrCreate(MFilterMutex);
-  return FilterMutex;
-}
-
 std::vector<adapter_impl *> &GlobalHandler::getAdapters() {
   static std::vector<adapter_impl *> &adapters = getOrCreate(MAdapters);
   enableOnCrashStackPrinting();

sycl/source/detail/global_handler.hpp

@@ -63,7 +63,6 @@ class GlobalHandler {
 
   std::mutex &getPlatformToDefaultContextCacheMutex();
   std::mutex &getPlatformMapMutex();
-  std::mutex &getFilterMutex();
   std::vector<adapter_impl *> &getAdapters();
   ods_target_list &getOneapiDeviceSelectorTargets(const std::string &InitValue);
   XPTIRegistry &getXPTIRegistry();
@@ -128,7 +127,6 @@ class GlobalHandler {
       MPlatformToDefaultContextCache;
   InstWithLock<std::mutex> MPlatformToDefaultContextCacheMutex;
   InstWithLock<std::mutex> MPlatformMapMutex;
-  InstWithLock<std::mutex> MFilterMutex;
   InstWithLock<std::vector<adapter_impl *>> MAdapters;
   InstWithLock<ods_target_list> MOneapiDeviceSelectorTargets;
   InstWithLock<XPTIRegistry> MXPTIRegistry;