Skip to content
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

[SYCL] Support negative filters for ONEAPI_DEVICE_SELECTOR #7309

Merged
merged 34 commits into from
Nov 10, 2022

Conversation

lbushi25
Copy link
Contributor

@lbushi25 lbushi25 commented Nov 8, 2022

This PR aims to add support for negative filters for the ONEAPI_DEVICE_SELECTOR variable to provide the user with a more flexible way of specifying which devices should and should not be available for usage. For example, ONEAPI_DEVICE_SELECTOR='opencl:*;!opencl:gpu' considers all opencl backend devices except for those that are of the gpu type.

@lbushi25 lbushi25 requested a review from gmlueck November 8, 2022 18:12
@bso-intel
Copy link
Contributor

@RaviNarayanaswamy ,
Please review the documentation part about the negative filter from the OMP's perspective.

@bso-intel bso-intel marked this pull request as ready for review November 8, 2022 19:05
@bso-intel bso-intel requested review from a team as code owners November 8, 2022 19:05
@bso-intel
Copy link
Contributor

@lbushi25 ,
If you enforced the order of filters (positive first then negative), please update the documentation.

@lbushi25
Copy link
Contributor Author

lbushi25 commented Nov 8, 2022

@bso-intel Yes, the documentation has been updated!

Furthermore, if the value of this environment variable only has discarding filters, an accepting filter that matches all devices, but not sub-devices and sub-sub-devices, will be implicitly included in the
environment variable to allow the user to specify only the list of devices that must not be made available. Therefore, `!*:cpu` will accept all devices except those that are of the cpu type and `opencl:*;!*:cpu`
will accept all devices of the opencl backend exept those that are of the opencl backend and of the cpu type. Finally, it is valid to have no accepting filter match a device but have at least one
discarding filter match the device. In this case, the device remains unavailable as expected.

The following examples further illustrate the usage of this environment variable:

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add the exclamation mark ('!') in this bullet.

  • The semi-colon character ( ; ) is treated specially by many shells, so you may need to enclose the string in quotes if the selection string contains this character.

Copy link
Contributor

@gmlueck gmlueck left a comment

Choose a reason for hiding this comment

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

Doc changes LGTM.

@RaviNarayanaswamy
Copy link
Contributor

@RaviNarayanaswamy , Please review the documentation part about the negative filter from the OMP's perspective.

LGTM

@lbushi25
Copy link
Contributor Author

lbushi25 commented Nov 9, 2022

The pre-commit failures are not related to this PR. The Regression/device_num.cpp failure will be fixed by #7336 of @raaiq1 and tanh_fix_test.cpp is not related to my changes and it seems to have been caused by an llvm-test-suite commit previously.

Copy link
Contributor

@cperkinsintel cperkinsintel left a comment

Choose a reason for hiding this comment

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

One question, but otherwise LGTM.

@lbushi25
Copy link
Contributor Author

pinging @intel/dpcpp-doc-reviewers @KseniyaTikhomirova for reachability since it has been open for a couple of days now.

@lbushi25
Copy link
Contributor Author

lbushi25 commented Nov 10, 2022 via email

@pvchupin pvchupin merged commit 6bd5f9c into intel:sycl Nov 10, 2022
@pvchupin
Copy link
Contributor

SYCL :: Basic/barrier_order.cpp test on HIP is disabled at intel/llvm-test-suite#1375

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.

6 participants