-
Notifications
You must be signed in to change notification settings - Fork 29
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
Update Level Zero loader detection for newer compiler builds #2025
base: master
Are you sure you want to change the base?
Conversation
SYCL nightly and future builds now link libur_adapter_level_zero to the level zero loader statically, so detecting through readelf is no longer possible For this case, we instead set the default Level Zero Loader filename to be 'libze_loader.so.1,' which is likely to not change See: intel/llvm#17104
@sommerlukas The only thing left is to make sure it works on Windows with nightly compiler |
View rendered docs @ https://intelpython.github.io/dpctl/pulls/2025/index.html |
Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_59 ran successfully. |
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.
Solution looks good, we could additionally allow users to set the variable.
Allows users to set Level Zero loader filename
Array API standard conformance tests for dpctl=0.20.0dev0=py310h93fe807_68 ran successfully. |
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.
Thank you @ndgrigorian, LGTM! Just a small nit.
endif() | ||
else () | ||
set(LIBZE_LOADER_FILENAME ${LIBZE_SET_LOADER_FILENAME}) | ||
endif() | ||
set(LIBZE_LOADER_FILENAME ${LIBZE_LOADER_FILENAME} PARENT_SCOPE) |
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.
Should we remove that line? It seems duplicating with one below.
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.
Difficult to say, it does duplicate, but it was already present in the code before, so I would want to be sure that it never served a purpose
As of intel/llvm#17104
libur_adapter_level_zero.so
is statically linked to the Level Zero loader, meaning that the shared object can no longer be read for the loader filenameTo resolve this, a default filename is now set in CMake, as it is unlikely to change. For backwards compatibility, it is only set to the default value if the object file is read and the loader is not found through the original approach.
Resolves #2022