-
Notifications
You must be signed in to change notification settings - Fork 115
[CMAKE] removing ONEDPL_DEVICE_TYPE and ONEDPL_DEVICE_BACKEND #2490
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Dan Hoeflinger <[email protected]>
Signed-off-by: Dan Hoeflinger <[email protected]>
Signed-off-by: Dan Hoeflinger <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes the confusing CMake configuration options ONEDPL_DEVICE_TYPE and ONEDPL_DEVICE_BACKEND that only controlled device filtering for ctest but not manual test execution. The change encourages users to directly use the ONEAPI_DEVICE_SELECTOR environment variable for consistent device selection across all test execution methods.
- Removed CMake logic for
ONEDPL_DEVICE_TYPEandONEDPL_DEVICE_BACKENDconfiguration options - Updated documentation to reference
ONEAPI_DEVICE_SELECTORinstead of deprecated device type variables - Modified CI workflows to use environment variable approach instead of CMake configuration
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/CMakeLists.txt | Removed device selection logic that conditionally set environment variables for tests |
| documentation/library_guide/onedpl_gsg.rst | Updated device selection guidance to use ONEAPI_DEVICE_SELECTOR |
| documentation/library_guide/introduction/onedpl_gsg.rst | Updated device selection guidance to use ONEAPI_DEVICE_SELECTOR |
| cmake/README.md | Removed documentation for deprecated ONEDPL_DEVICE_TYPE and ONEDPL_DEVICE_BACKEND options |
| CONTRIBUTING.md | Updated build instructions to use environment variable approach |
| CMakeLists.txt | Replaced device type/backend logic with deprecation warnings |
| .github/workflows/ci-testing.yml | Updated CI to set ONEAPI_DEVICE_SELECTOR instead of using CMake options |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Signed-off-by: Dan Hoeflinger <[email protected]>
Signed-off-by: Dan Hoeflinger <[email protected]>
Signed-off-by: Dan Hoeflinger <[email protected]>
Signed-off-by: Dan Hoeflinger <[email protected]>
Signed-off-by: Dan Hoeflinger <[email protected]>
Signed-off-by: Dan Hoeflinger <[email protected]>
Signed-off-by: Dan Hoeflinger <[email protected]>
Signed-off-by: Dan Hoeflinger <[email protected]>
Signed-off-by: Dan Hoeflinger <[email protected]>
Signed-off-by: Dan Hoeflinger <[email protected]>
dmitriy-sobolev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Removes ONEDPL_DEVICE_TYPE and ONEDPL_DEVICE_BACKEND settings from cmake, and triggers a fatal error if they are attempted to be used (suggesting instead the ONEAPI_DEVICE_SELECTOR environment variable equivalent to their proposed settings).
ONEDPL_DEVICE_TYPE and ONEDPL_DEVICE_BACKEND settings in CMake are confusing and may be misleading users about when they are and are not applied.
Before this PR, they would properly control the device filters when using ctest, but not when manually running the generated tests. It is not a compiler setting, but rather an environment variable for the runtime to adhere to. This also means if someone were to set their own ONEAPI_DEVICE_SELECTOR environment variable, it would be overridden by the ctest settings.
It is better for users to just directly use ONEAPI_DEVICE_SELECTOR.
I've made the decision also to only describe ONEAPI_DEVICE_SELECTOR and not SYCL_DEVICE_FILTER (which was used in the compiler pre 2023.1).