-
Notifications
You must be signed in to change notification settings - Fork 795
[SYCL][libclc] Fix cross builds #16244
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
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 seems to duplicate some work from #16182 - is that right?
Do we know why llvm/llvm-project#97811 and llvm/llvm-project#97392 weren't pulled down correctly? Were those commits just silently dropped?
Yes, it looks like this includes the changes from #16182.
I suspect the many non-trivial differences between upstream LLVM and DPC++ in libclc's CMake made it easy to miss what did and didn't need to be updated. |
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.
Libclc and runtime-related changes LGTM, but I agree with upstreaming as much as possible. 😉
When performing cross builds, we need native versions of various tools, we cannot assume the cross builds that are part of the current build are executable. LLVM provides the setup_host_tool function to handle this, either picking up versions of tools from LLVM_NATIVE_TOOL_DIR, or implicitly building native versions as needed. Use this in more places. This applies the changes from LLVM #97392 and #97811 and adapts them to DPC++, and makes the same changes in other places that are only needed for DPC++.
4387ef1 to
e152215
Compare
|
@intel/llvm-gatekeepers This should be good to merge, thanks. For upstreaming, it looks like upstream LLVM will want these changes only when they also get |
When performing cross builds, we need native versions of various tools, we cannot assume the cross builds that are part of the current build are executable. LLVM provides the setup_host_tool function to handle this, either picking up versions of tools from LLVM_NATIVE_TOOL_DIR, or implicitly building native versions as needed. Use this in more places.
This applies the changes from LLVM #97392 and #97811 and adapts them to DPC++, and makes the same changes in other places that are only needed for DPC++.
Tested with a suitable cross compilation toolchain file: