Skip to content

[UR] Make ONEAPI_DEVICE_SELECTOR prefilter test pass #17667

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 1 commit into from
Apr 24, 2025

Conversation

RossBrunton
Copy link
Contributor

The prefilter.cpp test was failing, this fixes it as follows:

  • Strings that can't be parsed are now treated as *:* rather than
    !*:*.
  • Backend names are validated; any invalid ones cause the entire string
    to be rejected.
  • In ur_lib.cpp, the parser is updated to handle failing to parse the
    string (doesn't affect the test, but a nice to have).
  • Some selector strings in the test itself were invalid and updated.

This was done according to the specification of ONEAPI_DEVICE_SELECTOR
located at https://github.com/intel/llvm/blob/sycl/sycl/doc/EnvironmentVariables.md#oneapi_device_selector .

@RossBrunton RossBrunton requested a review from a team as a code owner March 26, 2025 16:40
@RossBrunton
Copy link
Contributor Author

@nrspruit Can you have a quick look at the testing changes? I've updated the tests to be more inline with the format expected in Oneapi itself.

@RossBrunton
Copy link
Contributor Author

@nrspruit Mind having a look at this when you get the chance?

@RossBrunton
Copy link
Contributor Author

@nrspruit Can you look at this?

The prefilter.cpp test was failing, this fixes it as follows:
* Strings that can't be parsed are now treated as `*:*` rather than
  `!*:*`.
* Backend names are validated; any invalid ones cause the entire string
  to be rejected.
* In ur_lib.cpp, the parser is updated to handle failing to parse the
  string (doesn't affect the test, but a nice to have).
* Some selector strings in the test itself were invalid and updated.

This was done according to the specification of ONEAPI_DEVICE_SELECTOR
located at https://github.com/intel/llvm/blob/sycl/sycl/doc/EnvironmentVariables.md#oneapi_device_selector .

These changes also meant fixing the urDeviceGetSelected.cpp
conformance test, since we now return `INVALID_VALUE` rather than
`UNKNOWN`.
Copy link
Contributor

@nrspruit nrspruit left a comment

Choose a reason for hiding this comment

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

LGTM, sorry for the delay

@RossBrunton
Copy link
Contributor Author

@intel/llvm-gatekeepers Please merge.

@kbenzie kbenzie merged commit daf1fa6 into intel:sycl Apr 24, 2025
34 checks passed
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.

3 participants