[SYCL] Stop deriving ext::oneapi::filter_selector from device_selector - #23053
Open
KornevNikita wants to merge 2 commits into
Open
[SYCL] Stop deriving ext::oneapi::filter_selector from device_selector#23053KornevNikita wants to merge 2 commits into
KornevNikita wants to merge 2 commits into
Conversation
device_selector is a deprecated SYCL 1.2.1 class that is going to be removed, so ext::oneapi::filter_selector shouldn't be tied to it. It only derived from device_selector because its operator() wasn't reentrant: filter_selector_impl accumulated per-invocation state (the number of devices seen and the relative device number matched so far), so the selector had to reset() itself between the invocations. That is what the select_device() override was for, and it is what the pre-existing TODO in device_selector.cpp asked to get rid of. Compute the set of matching devices once, when the selector is constructed, using the very same matching algorithm as before. operator() becomes a pure function: it rejects any device outside of that set and ranks the rest with default_selector_v. As a result: * filter_selector is an ordinary SYCL 2020 callable device selector, so the !std::is_base_of_v<ext::oneapi::filter_selector, DeviceSelector> special case is dropped from EnableIfSYCL2020DeviceSelectorInvocable and the device/platform/queue constructors accept it through the regular callable path; * GlobalHandler::getFilterMutex(), a global mutex that guarded the mutable state during select_device(), is removed; * the base class, and with it reset() and select_device(), are dropped under __INTEL_PREVIEW_BREAKING_CHANGES. There is nothing left for reset() to reset, and SYCL 2020 doesn't require a device selector to have either method - a device can be constructed from the selector directly. In the non-preview library both are kept, and marked __SYCL_DEPRECATED, so that its ABI is unchanged. Everything else is ABI-neutral: filter_selector_impl is an internal class held behind a shared_ptr, and the mangling of operator()/reset()/select_device() doesn't change as long as they stay virtual in the non-preview library. sycl_symbols_linux.dump regenerated from the built libsycl.so is byte-identical to the reference, sycl_symbols_windows.dump is untouched, and the vtable layouts dumped with -fdump-vtable-layouts are identical for both ext::oneapi::filter_selector and ONEAPI::filter_selector. interop-image-get-native-mem.cpp was the only in-tree user of select_device(), so construct the device from the selector there instead. That keeps the test working when the E2E suite is run with test-preview-mode=True. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
Existing tests passed. |
| Filter.MatchesSeen++; | ||
| filter_selector_impl::filter_selector_impl(const std::string &Input) { | ||
| std::vector<filter> Filters; | ||
| for (const std::string &Filter : detail::tokenize(Input, ",")) |
Contributor
There was a problem hiding this comment.
what is the purpose of the change of tokenize output iteration?
| void filter_selector::reset() const { impl->reset(); } | ||
|
|
||
| // filter_selectors not "Callable" | ||
| // because of the requirement that the filter_selector "reset()" itself |
Contributor
There was a problem hiding this comment.
could you please clarify where this requirement comes from? I don't see it in extension doc. I would like to better understand the requirements
KornevNikita
force-pushed
the
filter-selector-no-device-selector
branch
from
September 1, 2026 16:01
a4dd185 to
2c3358e
Compare
The previous commit dropped the !std::is_base_of_v<ext::oneapi::filter_selector, DeviceSelector> clause from EnableIfSYCL2020DeviceSelectorInvocable altogether. Restore it under #ifndef __INTEL_PREVIEW_BREAKING_CHANGES: filter_selector becomes a standalone callable selector only in the preview mode, so outside of it the exclusion still describes the intent, even though it is subsumed by the !std::is_base_of_v<device_selector, DeviceSelector> clause that follows (in the non-preview library filter_selector still derives from device_selector). That also means the bullet in the previous commit message saying the special case is gone applies to the preview mode only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
KornevNikita
force-pushed
the
filter-selector-no-device-selector
branch
from
September 1, 2026 16:39
2c3358e to
58bf7d4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
device_selectoris a deprecated SYCL 1.2.1 class that is going to be removed, soext::oneapi::filter_selectorshouldn't be tied to it.Compute the set of matching devices once, when the selector is constructed, using the very same matching algorithm as before.
operator()becomes a pure function: it rejects any device outside of that set and ranks the rest withdefault_selector_v. As a result:filter_selectoris an ordinary SYCL 2020 callable device selector, so the!std::is_base_of_v<ext::oneapi::filter_selector, DeviceSelector>special case is dropped fromEnableIfSYCL2020DeviceSelectorInvocableand the device/platform/queue constructors accept it through the regular callable path;GlobalHandler::getFilterMutex(), a global mutex that guarded the mutable state duringselect_device(), is removed;reset()andselect_device(), are dropped under__INTEL_PREVIEW_BREAKING_CHANGES. There is nothing left forreset()to reset, and SYCL 2020 doesn't require a device selector to have either method - a device can be constructed from the selector directly. In the non-preview library both are kept, and marked__SYCL_DEPRECATED, so that its ABI is unchanged.