-
Notifications
You must be signed in to change notification settings - Fork 798
[Driver][SYCL] Use LLVM-IR based device libraries for device linking #13604
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
This is try intel#2 for this - original change was suspected of breaking the device library builds. This change represents 2 changes. 1. Update the build to create LLVM-IR based device libraries. This is done by using the existing -fsycl-device-only functionality to create the device libraries that are used during device link. 2. Update the compiler driver to use the LLVM-IR based device libraries. When performing the device linking instead of performing a full unbundle of the device libraries and using the device binary that was extracted. This streamlines the device linking behaviors, completely removing the cumbersome unbundling step from the process, freeing up a number of exteral tool calls.
|
The AMD/HIp failure is unrelated and has been reported here (Thanks Yury Plyakhin) Thanks |
asudarsa
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. Thanks
|
My bad on this one, I've never seen the docker image get destroyed before! |
|
@intel/dpcpp-clang-driver-reviewers, @intel/llvm-reviewers-runtime, please review. The original change was reverted due to some build problems that started occurring about the same time as the original PR being merged. Turns out that these changes were not the offending issue and can be re-applied. |
hchilama
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.
Driver Changes LGTM
|
@intel/llvm-gatekeepers , please merge. AMD/HIP failure is not related. I see other PRs have it as well: Thanks |
steffenlarsen
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!
|
Would it make sense to bundle all of these Or is there some reason for keeping the devicelib objs split per TU? |
This is try #2 for this - original change was suspected of breaking the device library builds.
This change represents 2 changes.
This is done by using the existing -fsycl-device-only functionality to create the device libraries that are used during device link.
When performing the device linking instead of performing a full unbundle of the device libraries and using the device binary that was extracted.
This streamlines the device linking behaviors, completely removing the cumbersome unbundling step from the process, freeing up a number of exteral tool calls.