[SYCL][clang-linker-wrapper] Update --bitcode-library option for SYCL #20331
Conversation
… linking bitcode device libraries
This seems unnecessary. |
|
@bader I did try using these options, but I wasn't able to get them to work, although I'm unsure the reason. Let me try again see what went wrong and see if I can use these options |
Please, pay attention to the way the driver builds command line arguments for the clang-linker-wrapper. SYCL specific option accepts a list of library paths separated by coma, whereas existing options accept only one path. To pass multiple libraries the driver must pass multiple arguments: one per library. |
YuriPlyakhin
left a comment
YuriPlyakhin
left a comment
There was a problem hiding this comment.
I'll review, after comments from Alexey are addressed.
| return FileOrErr.takeError(); | ||
| InputFiles[*FileOrErr].push_back(std::move(*FileOrErr)); | ||
| } | ||
|
|
There was a problem hiding this comment.
This functionality is not used nor tested - searching for --bitcode-library, nothing comes up. The --bitcode-library seems to original came from upstream, but then was removed upstream, and never removed downstream. I deleted the old functionality and made --bitcode-library more SYCL specific (see linkDevice. The old functionality of --bitcode-library linked the user objects with the bitcode libraries in one link job. For SYCL, we need to link the bitcode libraries in a separated job with the --only-needed option.
jzc
changed the title
jzc
changed the title
|
Please, fix CI issues. |
YuriPlyakhin
left a comment
YuriPlyakhin
left a comment
There was a problem hiding this comment.
LGTM. There are just a few minor nits.
maarquitos14
left a comment
maarquitos14
left a comment
There was a problem hiding this comment.
There are a couple of nits still unresolved from my review, but otherwise LGTM.
srividya-sundaram
left a comment
srividya-sundaram
left a comment
There was a problem hiding this comment.
Driver changes LGTM.
|
@intel/llvm-gatekeepers Failures are unrelated/flaky, this PR is ready to merge. |
6 participants
This PR updates the
--bitcode-libraryoption inclang-linker-wrapperto link bitcode device libraries that are used in NVIDIA and AMD GPU targets in SYCL offloading. This option had some previous behavior, but it was unused, so I deleted it and repurposed it for SYCL.SYCL::getDeviceLibraries()returns both fat objects and bitcode device libraries mixed together, while the-sycl-device-librariesoption only handles fat objects. This was causingclang-linker-wrapperto not link thelibdevicefor nvptx/AMD targets.This PR also removes the
-sycl-nvptx-device-librariesoption, which seemed to be doing the same thing but only for nvptx targets.