-
Notifications
You must be signed in to change notification settings - Fork 769
[SYCL] Use the system installed vc-intrinsics if available #18206
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
Conversation
Signed-off-by: Sarnie, Nick <[email protected]>
Signed-off-by: Sarnie, Nick <[email protected]>
|
||
# Date: Mar 7, 2025 | ||
# Update Triple usage after 979c275097 |
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.
The diff kinda sucks, all I did is wrap all the existing code in
find_package(LLVMGenXIntrinsics QUIET)
if (NOT LLVMGenXIntrinsics_FOUND)
endif()
I think it's possible our dev machines could have some obsolete installations. Can we provide an opt-in to download even if there is an installation present? Or, if the version is important, maybe using the installed an opt-in instead? That would help the folks who requested the change still. |
@aelovikov-intel I don't think we ever installed vc-intrinsics at the system level but sure I can do that |
I expect we'd want to apply the same logic for other dependencies, so even if one of them could have been installed before, it would be a good change. |
Signed-off-by: Sarnie, Nick <[email protected]>
@aelovikov-intel We already have two vars to specify custom source/include dirs to use, so I just wrapped the new check to not automatically search if those vars are set. |
For all external dependencies, we should try using the system installed one first and only download the repo if we can't find it.
Apply that for vc-intrinics first, I manually tested this with both static and shared library builds.