Skip to content

[SYCL] Remove outdated & invalid device score boost #5349

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.

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 2 commits into from
Feb 7, 2022

Conversation

KseniyaTikhomirova
Copy link
Contributor

@KseniyaTikhomirova KseniyaTikhomirova commented Jan 20, 2022

Every device that satisfies filter options receives 1000 as starting score.
Device that doesn't satisfy filter settings is filtered out on get_devices step. So no need to do extra boost for devices which DeviceNum is equal to the one specified in filter.

Every device that satisfies filter options receives 1000 as starting score.
Device that doesn't satisfy filter settings is filtered out on get_devices step.So no need to do extra boost for devices which DeviceNum is equal to the one specified in filter.

Signed-off-by: Tikhomirova, Kseniya <[email protected]>
device::get_devices returns devices that satisfies SYCL_DEVICE_FILTER.
extension filter_selector is also based on get_devices and just add extra level of device filtering.
no way to get device that do not satisfy SYCL_DEVICE_FILTER in runtim so that operator() have to do
filter check
@romanovvlad
Copy link
Contributor

LGTM
@bso-intel Could you please take a look?

@bader bader changed the title [SYCL] remove outdated & invalid device score boost [SYCL] Remove outdated & invalid device score boost Jan 20, 2022
Copy link
Contributor

@s-kanaev s-kanaev left a comment

Choose a reason for hiding this comment

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

Should there be any tests updated or added?

@bso-intel bso-intel self-requested a review January 20, 2022 23:05
Copy link
Contributor

@bso-intel bso-intel left a comment

Choose a reason for hiding this comment

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

LGTM.
Yes, we don't need this legacy filtering code since we already filter devices in platform_impl::filterDeviceFilter. Thanks.

@KseniyaTikhomirova
Copy link
Contributor Author

Should there be any tests updated or added?

working on test rework right now

@KseniyaTikhomirova
Copy link
Contributor Author

uploaded PR for test intel/llvm-test-suite#775

@s-kanaev
Copy link
Contributor

s-kanaev commented Feb 2, 2022

/verify with intel/llvm-test-suite#786

@KseniyaTikhomirova
Copy link
Contributor Author

/verify with intel/llvm-test-suite#775

@KseniyaTikhomirova
Copy link
Contributor Author

Hi @vladimirlaz, this PR passed checks with the corresponding change in test suite. Could you please review and merge if no concerns? Thank you.

@vladimirlaz
Copy link
Contributor

Will be merged later today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants