Skip to content

Commit

Permalink
[SYCL] Fix invalid input handling in ONEAPI_DEVICE_SELECTOR (#12299)
Browse files Browse the repository at this point in the history
Cover cases when backend and/or devices are not specified, when backend
or device name is a substring (we expect exact match according to
documentation) etc.
  • Loading branch information
againull authored Jan 8, 2024
1 parent ea0e82b commit 90b6aee
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 48 deletions.
47 changes: 34 additions & 13 deletions sycl/source/detail/device_filter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ inline namespace _V1 {
namespace detail {

std::vector<std::string_view> tokenize(const std::string_view &Filter,
const std::string &Delim) {
const std::string &Delim,
bool ProhibitEmptyTokens = false) {
std::vector<std::string_view> Tokens;
size_t Pos = 0;
size_t LastPos = 0;
Expand All @@ -30,6 +31,11 @@ std::vector<std::string_view> tokenize(const std::string_view &Filter,

if (!Tok.empty()) {
Tokens.push_back(Tok);
} else if (ProhibitEmptyTokens) {
throw sycl::exception(
sycl::make_error_code(errc::invalid),
"ONEAPI_DEVICE_SELECTOR parsing error. Empty input before '" + Delim +
"' delimiter is not allowed.");
}
// move the search starting index
LastPos = Pos + 1;
Expand All @@ -39,6 +45,12 @@ std::vector<std::string_view> tokenize(const std::string_view &Filter,
if (LastPos < Filter.size()) {
std::string_view Tok(Filter.data() + LastPos, Filter.size() - LastPos);
Tokens.push_back(Tok);
} else if ((LastPos != 0) && ProhibitEmptyTokens) {
// if delimiter is the last sybmol in the string.
throw sycl::exception(
sycl::make_error_code(errc::invalid),
"ONEAPI_DEVICE_SELECTOR parsing error. Empty input after '" + Delim +
"' delimiter is not allowed.");
}
return Tokens;
}
Expand All @@ -52,10 +64,9 @@ static backend Parse_ODS_Backend(const std::string_view &BackendStr,
auto SyclBeMap =
getSyclBeMap(); // <-- std::array<std::pair<std::string, backend>>
// [{"level_zero", backend::level_zero}, {"*", ::all}, ...
auto It = std::find_if(
std::begin(SyclBeMap), std::end(SyclBeMap), [&](auto BePair) {
return std::string::npos != BackendStr.find(BePair.first);
});
auto It =
std::find_if(std::begin(SyclBeMap), std::end(SyclBeMap),
[&](auto BePair) { return BackendStr == BePair.first; });

if (It == SyclBeMap.end()) {
// backend is required
Expand All @@ -72,17 +83,22 @@ static backend Parse_ODS_Backend(const std::string_view &BackendStr,
static void Parse_ODS_Device(ods_target &Target,
const std::string_view &DeviceStr) {
// DeviceStr will be: 'gpu', '*', '0', '0.1', 'gpu.*', '0.*', or 'gpu.2', etc.
std::vector<std::string_view> DeviceSubTuple = tokenize(DeviceStr, ".");
std::vector<std::string_view> DeviceSubTuple =
tokenize(DeviceStr, ".", true /* ProhibitEmptyTokens */);
if (DeviceSubTuple.empty())
throw sycl::exception(
sycl::make_error_code(errc::invalid),
"ONEAPI_DEVICE_SELECTOR parsing error. Device must be specified.");

std::string_view TopDeviceStr = DeviceSubTuple[0];

// Handle explicit device type (e.g. 'gpu').
auto DeviceTypeMap =
getSyclDeviceTypeMap(); // <-- std::array<std::pair<std::string,
// info::device::type>>
auto It = std::find_if(
std::begin(DeviceTypeMap), std::end(DeviceTypeMap), [&](auto DtPair) {
return std::string::npos != TopDeviceStr.find(DtPair.first);
});
auto It =
std::find_if(std::begin(DeviceTypeMap), std::end(DeviceTypeMap),
[&](auto DtPair) { return TopDeviceStr == DtPair.first; });
if (It != DeviceTypeMap.end()) {
Target.DeviceType = It->second;
// Handle wildcard.
Expand Down Expand Up @@ -183,15 +199,20 @@ Parse_ONEAPI_DEVICE_SELECTOR(const std::string &envString) {
// Each entry: "level_zero:gpu" or "opencl:0.0,0.1" or "opencl:*" but NOT just
// "opencl".
for (const auto Entry : Entries) {
std::vector<std::string_view> Pair = tokenize(Entry, ":");
backend be = Parse_ODS_Backend(Pair[0], Entry); // Pair[0] is backend.
std::vector<std::string_view> Pair =
tokenize(Entry, ":", true /* ProhibitEmptyTokens */);

if (Pair.size() == 1) {
if (Pair.empty()) {
std::stringstream ss;
ss << "Incomplete selector! Backend and device must be specified.";
throw sycl::exception(sycl::make_error_code(errc::invalid), ss.str());
} else if (Pair.size() == 1) {
std::stringstream ss;
ss << "Incomplete selector! Try '" << Pair[0]
<< ":*' if all devices under the backend was original intention.";
throw sycl::exception(sycl::make_error_code(errc::invalid), ss.str());
} else if (Pair.size() == 2) {
backend be = Parse_ODS_Backend(Pair[0], Entry); // Pair[0] is backend.
std::vector<std::string_view> Targets = tokenize(Pair[1], ",");
for (auto TargetStr : Targets) {
ods_target DeviceTarget(be);
Expand Down
48 changes: 24 additions & 24 deletions sycl/test-e2e/Config/select_device.cpp
Original file line number Diff line number Diff line change
@@ -1,41 +1,41 @@
// REQUIRES: gpu
// RUN: %{build} -o %t.out
//
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out DEVICE_INFO write > %t.txt
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out DEVICE_INFO read %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out DEVICE_INFO write > %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out DEVICE_INFO read %t.txt
//
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out PLATFORM_INFO write > %t.txt
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out PLATFORM_INFO read %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out PLATFORM_INFO write > %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out PLATFORM_INFO read %t.txt
//
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out DEVICE_ERROR_INFO write > %t.txt
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out DEVICE_ERROR_INFO read %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out DEVICE_ERROR_INFO write > %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out DEVICE_ERROR_INFO read %t.txt
//
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out PLATFORM_ERROR_INFO write > %t.txt
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out PLATFORM_ERROR_INFO read %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out PLATFORM_ERROR_INFO write > %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out PLATFORM_ERROR_INFO read %t.txt
//
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out REG_EX_INFO write > %t.txt
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out REG_EX_INFO read %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out REG_EX_INFO write > %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out REG_EX_INFO read %t.txt
//
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out DEVICE_NAME_INFO write > %t.txt
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out DEVICE_NAME_INFO read %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out DEVICE_NAME_INFO write > %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out DEVICE_NAME_INFO read %t.txt
//
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out PLATFORM_NAME_INFO write > %t.txt
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out PLATFORM_NAME_INFO read %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out PLATFORM_NAME_INFO write > %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out PLATFORM_NAME_INFO read %t.txt
//
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out DEVICE_MULTI_INFO write > %t.txt
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out DEVICE_MULTI_INFO read %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out DEVICE_MULTI_INFO write > %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out DEVICE_MULTI_INFO read %t.txt
//
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out DEVICE_MALFORMED_INFO write > %t.txt
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out DEVICE_MALFORMED_INFO read %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out DEVICE_MALFORMED_INFO write > %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out DEVICE_MALFORMED_INFO read %t.txt
//
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out PLATOFRM_MALFORMED_INFO write > %t.txt
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out PLATOFRM_MALFORMED_INFO read %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out PLATOFRM_MALFORMED_INFO write > %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out PLATOFRM_MALFORMED_INFO read %t.txt
//
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out DRIVER_MALFORMED_INFO write > %t.txt
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out DRIVER_MALFORMED_INFO read %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out DRIVER_MALFORMED_INFO write > %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out DRIVER_MALFORMED_INFO read %t.txt
//
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out PLATVER_MALFORMED_INFO write > %t.txt
// RUN: env "ONEAPI_DEVICE_SELECTOR=\*:gpu" %{run-unfiltered-devices} %t.out PLATVER_MALFORMED_INFO read %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out PLATVER_MALFORMED_INFO write > %t.txt
// RUN: env ONEAPI_DEVICE_SELECTOR="*:gpu" %{run-unfiltered-devices} %t.out PLATVER_MALFORMED_INFO read %t.txt
//

//==------------ select_device.cpp - SYCL_DEVICE_ALLOWLIST test ------------==//
Expand Down
7 changes: 0 additions & 7 deletions sycl/test-e2e/OneapiDeviceSelector/illegal_BE_error.cpp

This file was deleted.

15 changes: 15 additions & 0 deletions sycl/test-e2e/OneapiDeviceSelector/illegal_input.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@

// RUN: %clangxx -fsycl -fsycl-targets=%{sycl_triple} %S/Inputs/trivial.cpp -o %t.out
// RUN: env ONEAPI_DEVICE_SELECTOR="macaroni:*" %{run-unfiltered-devices} %t.out
// RUN: env ONEAPI_DEVICE_SELECTOR=":" %{run-unfiltered-devices} %t.out
// RUN: env ONEAPI_DEVICE_SELECTOR="level_zero:." %{run-unfiltered-devices} %t.out
// RUN: env ONEAPI_DEVICE_SELECTOR="macaroni_level_zero:." %{run-unfiltered-devices} %t.out
// RUN: env ONEAPI_DEVICE_SELECTOR="level_zero:macaroni_gpu" %{run-unfiltered-devices} %t.out
// RUN: env ONEAPI_DEVICE_SELECTOR="level_zero:0..0" %{run-unfiltered-devices} %t.out
// RUN: env ONEAPI_DEVICE_SELECTOR="level_zero:" %{run-unfiltered-devices} %t.out
// RUN: env ONEAPI_DEVICE_SELECTOR="level_zero:::gpu" %{run-unfiltered-devices} %t.out
// RUN: env ONEAPI_DEVICE_SELECTOR="level_zero:.1" %{run-unfiltered-devices} %t.out
// XFAIL: *

// Calling ONEAPI_DEVICE_SELECTOR with an illegal input should result in an
// error.
6 changes: 3 additions & 3 deletions sycl/test-e2e/Plugin/sycl-ls-unified-runtime.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// REQUIRES: gpu, level_zero
// RUN: env ONEAPI_DEVICE_SELECTOR="ext_oneapi_level_zero:*" sycl-ls 2>&1 | FileCheck --check-prefixes=CHECK-PI %s
// RUN: env SYCL_PREFER_UR=0 ONEAPI_DEVICE_SELECTOR="ext_oneapi_level_zero:*" sycl-ls 2>&1 | FileCheck --check-prefixes=CHECK-PI %s
// RUN: env SYCL_PI_TRACE=-1 SYCL_PREFER_UR=1 ONEAPI_DEVICE_SELECTOR="ext_oneapi_level_zero:*" sycl-ls 2>&1 | FileCheck --check-prefixes=CHECK-UR %s
// RUN: env ONEAPI_DEVICE_SELECTOR="level_zero:*" sycl-ls 2>&1 | FileCheck --check-prefixes=CHECK-PI %s
// RUN: env SYCL_PREFER_UR=0 ONEAPI_DEVICE_SELECTOR="level_zero:*" sycl-ls 2>&1 | FileCheck --check-prefixes=CHECK-PI %s
// RUN: env SYCL_PI_TRACE=-1 SYCL_PREFER_UR=1 ONEAPI_DEVICE_SELECTOR="level_zero:*" sycl-ls 2>&1 | FileCheck --check-prefixes=CHECK-UR %s

// CHECK-PI: Intel(R) Level-Zero
// CHECK-UR: Intel(R) oneAPI Unified Runtime over Level-Zero
Expand Down
2 changes: 1 addition & 1 deletion sycl/test/Unit/lit.cfg.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def find_shlibpath_var():
# The mock plugin currently appears as an opencl plugin, but could be changed in
# the future. To avoid it being filtered out we set the filter to use the *
# wildcard.
config.environment["ONEAPI_DEVICE_SELECTOR"] = "'*:*'"
config.environment["ONEAPI_DEVICE_SELECTOR"] = "*:*"
lit_config.note("Using Mock Plugin.")

config.environment["SYCL_CACHE_DIR"] = config.llvm_obj_root + "/sycl_cache"
Expand Down

0 comments on commit 90b6aee

Please sign in to comment.