-
Notifications
You must be signed in to change notification settings - Fork 32
Fix CMake error in OS compiler workflow #2211
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: master
Are you sure you want to change the base?
Conversation
|
View rendered docs @ https://intelpython.github.io/dpctl/pulls/2211/index.html |
b4097f7 to
4be2052
Compare
4be2052 to
bed074b
Compare
|
Array API standard conformance tests for dpctl=0.22.0dev0=py310h93fe807_88 ran successfully. |
1 similar comment
|
Array API standard conformance tests for dpctl=0.22.0dev0=py310h93fe807_88 ran successfully. |
|
Array API standard conformance tests for dpctl=0.22.0dev0=py310h93fe807_88 ran successfully. |
| MATH(EXPR IDX "${IDX}+1") | ||
| endforeach() | ||
| list(GET IntelSyclCompiler_VERSION_LIST 0 VERSION_STRING) | ||
| if("${VERSION_STRING}" MATCHES "Intel SYCL compiler Nightly") |
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.
Would it make sense to update the libsyclinterface/cmake/modules/FindIntelSyclCompiler.cmake with the latest content from the recent 2025.3 release instead?
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.
I see what you're saying here now. This file isn't a vendored CMake module, this is a custom CMake module for finding the compiler that consolidated old logic for finding the compiler.
That is, 2025.3 would contain no equivalent file to update with, if that's what you're saying
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.
I will say that the solution here is a bit hacky, a more elegant solution may be to check each index instead, and get the index that's either the DPC++ version or the clang version.
This would future-proof any other, future changes to how the versioning is given
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.
This file isn't a vendored CMake module, this is a custom CMake module for finding the compiler that consolidated old logic for finding the compiler.
I see, it seems my previous understanding wasn't correct.
I thought it is exactly the same as supplied by DPC++ compiler conda package but not available by expected due to some reason or other deployment.
antonwolfy
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.
No more comments from me. LGTM!
This PR fixes the failing OS compiler workflow, which has been throwing a CMake error
OS compiler versioning output changed, so the Clang version is preceded by another string giving the date and specifying the compiler is the SYCL nightly compiler
This PR modifies dpctl CMake to check if the "Intel SYCL Compiler Nightly" string is in the version string. If so, the next member of the list, containing the Clang version, is used instead