Skip to content

[SYCL] Don't select devices with no available images #6203

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jun 7, 2022
Merged

[SYCL] Don't select devices with no available images #6203

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
326f40a
[SYCL] Don't select devices with no available images
npmiller May 25, 2022
86483b9
[SYCL] Refactor device selector
npmiller May 25, 2022
01d278e
[SYCL] Improve hasCompatibleImage
npmiller May 27, 2022
17305b1
[SYCL] Remove unused program_manager
npmiller May 27, 2022
f19c374
[SYCL] Use device preference for tie breaking
npmiller May 30, 2022
e9257cf
[SYCL] Reverse pref order for readability
npmiller May 30, 2022
5d91dec
[SYCL] Boost score of device type filter
npmiller Jun 1, 2022
a36d209
[SYCL] Don't reject devices from default selector
npmiller Jun 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions sycl/source/detail/program_manager/program_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1450,6 +1450,16 @@ kernel_id ProgramManager::getSYCLKernelID(const std::string &KernelName) {
return KernelID->second;
}

bool ProgramManager::hasCompatibleImage(const device &Dev) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe instead of running two loops and creating a temporary set, would it be possible to use std::find or std::any_of on the DeviceImages collection ? That might be easier to read and quicker.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated with a slightly easier to use map and use of std::any_of

std::lock_guard<std::mutex> Guard(m_KernelIDsMutex);

return std::any_of(
m_BinImg2KernelIDs.cbegin(), m_BinImg2KernelIDs.cend(),
[&](std::pair<RTDeviceBinaryImage *,
std::shared_ptr<std::vector<kernel_id>>>
Elem) { return compatibleWithDevice(Elem.first, Dev); });
}

std::vector<kernel_id> ProgramManager::getAllSYCLKernelIDs() {
std::lock_guard<std::mutex> KernelIDsGuard(m_KernelIDsMutex);

Expand Down
3 changes: 3 additions & 0 deletions sycl/source/detail/program_manager/program_manager.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,9 @@ class ProgramManager {
void addOrInitDeviceGlobalEntry(const void *DeviceGlobalPtr,
const char *UniqueId);

// Returns true if any available image is compatible with the device Dev.
bool hasCompatibleImage(const device &Dev);

// The function returns a vector of SYCL device images that are compiled with
// the required state and at least one device from the passed list of devices.
std::vector<device_image_plain> getSYCLDeviceImagesWithCompatibleState(
Expand Down
78 changes: 35 additions & 43 deletions sycl/source/device_selector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,27 @@
__SYCL_INLINE_NAMESPACE(cl) {
namespace sycl {

// Utility function to check if device is of the preferred backend.
// Currently preference is given to the level_zero backend.
static bool isDeviceOfPreferredSyclBe(const device &Device) {
// SYCL_DEVICE_FILTER doesn't need to be considered in the device preferences
// as it filters the device list returned by device::get_devices itself, so
// only matching devices will be scored.
static int getDevicePreference(const device &Device) {
int Score = 0;

// No preferences for host devices.
if (Device.is_host())
return false;
return Score;

// Strongly prefer devices with available images.
auto &program_manager = cl::sycl::detail::ProgramManager::getInstance();
if (program_manager.hasCompatibleImage(Device))
Score += 1000;

// Prefer level_zero backend devices.
if (detail::getSyclObjImpl(Device)->getPlugin().getBackend() ==
backend::ext_oneapi_level_zero)
Score += 50;

return detail::getSyclObjImpl(Device)->getPlugin().getBackend() ==
backend::ext_oneapi_level_zero;
return Score;
}

device device_selector::select_device() const {
Expand Down Expand Up @@ -64,11 +77,12 @@ device device_selector::select_device() const {

// SYCL spec says: "If more than one device receives the high score then
// one of those tied devices will be returned, but which of the devices
// from the tied set is to be returned is not defined". Here we give a
// preference to the device of the preferred BE.
//
// from the tied set is to be returned is not defined". So use the device
// preference score to resolve ties, this is necessary for custom_selectors
// that may not already include device preference in their scoring.
if ((score < dev_score) ||
(score == dev_score && isDeviceOfPreferredSyclBe(dev))) {
((score == dev_score) &&
(getDevicePreference(*res) < getDevicePreference(dev)))) {
res = &dev;
score = dev_score;
}
Expand Down Expand Up @@ -97,25 +111,13 @@ device device_selector::select_device() const {
/// 1. GPU
/// 2. CPU
/// 3. Host
/// 4. Accelerator
int default_selector::operator()(const device &dev) const {

int Score = REJECT_DEVICE_SCORE;

// Give preference to device of SYCL BE.
if (isDeviceOfPreferredSyclBe(dev))
Score = 50;

// If SYCL_DEVICE_FILTER is set, filter device gets a high point.
// All unmatched devices should never be selected.
detail::device_filter_list *FilterList =
detail::SYCLConfig<detail::SYCL_DEVICE_FILTER>::get();
// device::get_devices returns filtered list of devices.
// Keep 1000 for default score when filters were applied.
if (FilterList)
Score = 1000;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this removed? I am not completely sure, but it seems like this tries to ensure that no device is rejected if the user applied a filter. Maybe we could simply change it to a += 1 to ensure this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the history I got the feeling that it was more of a leftover from a time where the default selector was actually using the environment variable to do some filtering, and it felt a bit pointless since if there is a filter all the devices here would start at 1000 score, last PR changing this: #5349

You make a good point about a filtered device potentially being rejected, although I don't think it can happen at the moment since all defined device types are covered and add score, but what would you think about just starting the score for the default selector at 0 rather than REJECT_DEVICE_SCORE, instead of restoring this? I don't think it really makes sense for the default selector to reject any devices.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the patch as I suggested, let me know what you think

// The default selector doesn't reject any devices.
int Score = 0;

if (dev.get_info<info::device::device_type>() == detail::get_forced_type())
Score += 1000;
Score += 2000;

if (dev.is_gpu())
Score += 500;
Expand All @@ -132,18 +134,18 @@ int default_selector::operator()(const device &dev) const {
if (dev.is_accelerator())
Score += 75;

// Add preference score.
Score += getDevicePreference(dev);

return Score;
}

int gpu_selector::operator()(const device &dev) const {
int Score = REJECT_DEVICE_SCORE;

if (dev.is_gpu()) {
// device::get_devices returns filtered list of devices.
Score = 1000;
// Give preference to device of SYCL BE.
if (isDeviceOfPreferredSyclBe(dev))
Score += 50;
Score += getDevicePreference(dev);
}
return Score;
}
Expand All @@ -152,12 +154,8 @@ int cpu_selector::operator()(const device &dev) const {
int Score = REJECT_DEVICE_SCORE;

if (dev.is_cpu()) {
// device::get_devices returns filtered list of devices.
Score = 1000;

// Give preference to device of SYCL BE.
if (isDeviceOfPreferredSyclBe(dev))
Score += 50;
Score += getDevicePreference(dev);
}
return Score;
}
Expand All @@ -166,12 +164,8 @@ int accelerator_selector::operator()(const device &dev) const {
int Score = REJECT_DEVICE_SCORE;

if (dev.is_accelerator()) {
// device::get_devices returns filtered list of devices.
Score = 1000;

// Give preference to device of SYCL BE.
if (isDeviceOfPreferredSyclBe(dev))
Score += 50;
Score += getDevicePreference(dev);
}
return Score;
}
Expand All @@ -181,9 +175,7 @@ int host_selector::operator()(const device &dev) const {

if (dev.is_host()) {
Score = 1000;
// Give preference to device of SYCL BE.
if (isDeviceOfPreferredSyclBe(dev))
Score += 50;
Score += getDevicePreference(dev);
}
return Score;
}
Expand Down