[ClangLinkerWrapper] Reintroduce missing upstream commits that remove in-house LTO in clang-linker-wrapper #17323
Conversation
Summary: This should be the linker's job if the user creates any bitcode files, then passing `-flto` to the linker for the toolchain should be able to handle it. Right now this path is only used in the case where someone does LTO w/ ld.gold targeting a CPU so I think we are safe here as that will still be forwarded, for bfd it'll be an error as it would on the host. I think I talked the SYCL team out of using this as well so I should be good to delete it.
This patch fixes: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:642:6: error: unused function 'diagnosticHandler' [-Werror,-Wunused-function]
…159d95ca6d0823ea [LinkerWrapper] Pass all files to the device linker Summary: The linker wrapper's job is to extract embedded device code from fat binaries and create linked images that can then be embedded and executed. In order to support LTO, we originally reinvented all of the LTO handling that `ld.lld` normally does. Primarily, this was because `nvlink` didn't support this at all, and we have special hacks required for offloading languages interacting with archive libraries. Now since I wrote #96561 we should be able to pass all the inputs to the device linker transparently. This has the advantage of allowing the `clang` Driver to do its own handling. Primarily, this will be used to implicitly pass libraries to the device link job to make it more consistent with other toolchains. The JIT support is a notable departure, however there is an option called `--lto-emit-llvm` that performs the exact function where we want the final link job to output LLVM-IR that we can then embed instead. This patch does not fully delete the LTO handling, primarily because I think the SPIR-V people might want it. To see only the relevant patches, ignore the first commit of the nvlink-wrapper. Depends on #96561. Author: Joseph Huber
|
@asudarsa, I suggest we merge to |
Signed-off-by: Arvind Sudarsanam <[email protected]>
As we discussed offline, we can merge without squashing here as well. Thanks |
|
Test failure and format issue resolved in latest commit. Thanks |
|
Test failures do not seem to be related to my PR and seems like infrastructure issue. @intel/llvm-gatekeepers, can you please take a look and provide feedback? Thanks |
|
Quick ping: @AlexeySachkov and @ldrumm Thanks |
ldrumm
left a comment
ldrumm
left a comment
There was a problem hiding this comment.
Apologies for my tardiness in reviewing. This looks good, but please double check the early exit. I don't know enough about this part of the code to know whether that's intended or not
|
AMD failure seems like infrastructure issue. Thanks |
Review from @intel/dpcpp-tools-reviewers is missing. |
|
Can you please take a look? Thanks |
|
Hi @intel/llvm-gatekeepers This is ready for merge. Thanks |
Not infra issue, just too long delay between build and AMD test run, the artifacts expired |
9 participants
A couple of upstream commits were not merged due to merge conflicts.
This PR reintroduces the following upstream commits:
ccd73ee
4a96081
Also, This PR restores missing changes from upstream commit aae059e1389bebe86ceb3aea159d95ca6d0823ea](f2f1a14)
Thanks