[UR] Make ONEAPI_DEVICE_SELECTOR prefilter test pass (#17667)

The prefilter.cpp test was failing, this fixes it as follows:
* Strings that can't be parsed are now treated as `*:*` rather than
  `!*:*`.
* Backend names are validated; any invalid ones cause the entire string
  to be rejected.
* In ur_lib.cpp, the parser is updated to handle failing to parse the
  string (doesn't affect the test, but a nice to have).
* Some selector strings in the test itself were invalid and updated.

This was done according to the specification of ONEAPI_DEVICE_SELECTOR
located at
https://github.com/intel/llvm/blob/sycl/sycl/doc/EnvironmentVariables.md#oneapi_device_selector
.

Author: RossBrunton

unified-runtime/source/loader/ur_adapter_registry.hpp

@@ -234,7 +234,7 @@ class AdapterRegistry {
       // success.
       logger::error("ERROR: missing backend, format of filter = "
                     "'[!]backend:filterStrings'");
-      return UR_RESULT_SUCCESS;
+      odsEnvMap = std::nullopt;
     }
     logger::debug("getenv_to_map parsed env var and {} a map",
                   (odsEnvMap.has_value() ? "produced" : "failed to produce"));
@@ -245,20 +245,34 @@ class AdapterRegistry {
     EnvVarMap mapODS =
         odsEnvMap.has_value() ? odsEnvMap.value() : EnvVarMap{{"*", {"*"}}};
 
+    // Check all backends are valid backend names
+    for (auto entry : mapODS) {
+      if (entry.first == "*" || entry.first == "!*") {
+        continue;
+      }
+      auto check = [&](const ur_adapter_manifest &m) {
+        if (entry.first == m.name || entry.first == "!" + m.name) {
+          return true;
+        }
+        return false;
+      };
+      if (std::any_of(ur_adapter_manifests.begin(), ur_adapter_manifests.end(),
+                      check)) {
+        continue;
+      }
+
+      // Backend name is not legal, wipe the list
+      mapODS = EnvVarMap{{"*", {"*"}}};
+      break;
+    }
+
     std::vector<FilterTerm> positiveFilters;
     std::vector<FilterTerm> negativeFilters;
 
     for (auto &termPair : mapODS) {
       std::string backend = termPair.first;
       // TODO: Figure out how to process all ODS errors rather than returning
       // on the first error.
-      if (backend.empty()) {
-        // FIXME: never true because getenv_to_map rejects this case
-        // malformed term: missing backend -- output ERROR, then continue
-        logger::error("ERROR: missing backend, format of filter = "
-                      "'[!]backend:filterStrings'");
-        continue;
-      }
       logger::debug("ONEAPI_DEVICE_SELECTOR Pre-Filter with backend '{}' ",
                     backend);
 

unified-runtime/source/loader/ur_lib.cpp

@@ -330,9 +330,15 @@ ur_result_t urDeviceGetSelected(ur_platform_handle_t hPlatform,
   // (If we wished to preserve the ordering of terms, we could replace
   // `std::map` with `std::queue<std::pair<key_type_t, value_type_t>>` or
   // something similar.)
-  auto maybeEnvVarMap =
-      getenv_to_map("ONEAPI_DEVICE_SELECTOR", /* reject_empty= */ false,
-                    /* allow_duplicate= */ false, /* lower= */ true);
+  std::optional<EnvVarMap> maybeEnvVarMap{};
+  try {
+    maybeEnvVarMap =
+        getenv_to_map("ONEAPI_DEVICE_SELECTOR", /* reject_empty= */ false,
+                      /* allow_duplicate= */ false, /* lower= */ true);
+  } catch (...) {
+    logger::error("ERROR: could not parse ONEAPI_DEVICE_SELECTOR string");
+    return UR_RESULT_ERROR_INVALID_VALUE;
+  }
   logger::debug(
       "getenv_to_map parsed env var and {} a map",
       (maybeEnvVarMap.has_value() ? "produced" : "failed to produce"));

unified-runtime/test/conformance/device/urDeviceGetSelected.cpp

@@ -187,7 +187,7 @@ TEST_P(urDeviceGetSelectedTest, InvalidMissingBackend) {
   uur::set_env("ONEAPI_DEVICE_SELECTOR", ":garbage");
   uint32_t count = 0;
   ASSERT_EQ_RESULT(
-      UR_RESULT_ERROR_UNKNOWN,
+      UR_RESULT_ERROR_INVALID_VALUE,
       urDeviceGetSelected(platform, UR_DEVICE_TYPE_ALL, 0, nullptr, &count));
   ASSERT_EQ(count, 0);
 }
@@ -227,7 +227,7 @@ TEST_P(urDeviceGetSelectedTest, InvalidMissingFilterString) {
   uur::set_env("ONEAPI_DEVICE_SELECTOR", "*:0,,2");
   uint32_t count = 0;
   ASSERT_EQ_RESULT(
-      UR_RESULT_ERROR_UNKNOWN,
+      UR_RESULT_ERROR_INVALID_VALUE,
       urDeviceGetSelected(platform, UR_DEVICE_TYPE_ALL, 0, nullptr, &count));
   ASSERT_EQ(count, 0);
 }

unified-runtime/test/loader/adapter_registry/prefilter.cpp

@@ -63,7 +63,7 @@ TEST_F(adapterPreFilterTest, testPrefilterDiscardFilterMultipleBackends) {
 }
 
 TEST_F(adapterPreFilterTest, testPrefilterAcceptAndDiscardFilter) {
-  SetUp("!cuda:*;level_zero:*");
+  SetUp("level_zero:*;!cuda:*");
   auto levelZeroExists =
       std::any_of(registry->cbegin(), registry->cend(), haslevelzeroLibName);
   EXPECT_TRUE(levelZeroExists);
@@ -76,7 +76,7 @@ TEST_F(adapterPreFilterTest, testPrefilterAcceptAndDiscardFilter) {
 }
 
 TEST_F(adapterPreFilterTest, testPrefilterDiscardFilterAll) {
-  SetUp("*");
+  SetUp("*:*");
   auto levelZeroExists =
       std::any_of(registry->cbegin(), registry->cend(), haslevelzeroLibName);
   EXPECT_TRUE(levelZeroExists);
@@ -115,10 +115,10 @@ TEST_F(adapterPreFilterTest, testPrefilterWithInvalidBackend) {
 }
 
 TEST_F(adapterPreFilterTest, testPrefilterWithNotAllAndAcceptFilter) {
-  SetUp("!*;level_zero");
+  SetUp("level_zero:*;!*:*");
   auto levelZeroExists =
       std::any_of(registry->cbegin(), registry->cend(), haslevelzeroLibName);
-  EXPECT_TRUE(levelZeroExists);
+  EXPECT_FALSE(levelZeroExists);
   auto openclExists =
       std::any_of(registry->cbegin(), registry->cend(), hasOpenclLibName);
   EXPECT_FALSE(openclExists);
@@ -128,7 +128,7 @@ TEST_F(adapterPreFilterTest, testPrefilterWithNotAllAndAcceptFilter) {
 }
 
 TEST_F(adapterPreFilterTest, testPrefilterWithNotAllFilter) {
-  SetUp("!*");
+  SetUp("!*:*");
   auto levelZeroExists =
       std::any_of(registry->cbegin(), registry->cend(), haslevelzeroLibName);
   EXPECT_FALSE(levelZeroExists);