[SYCL][NFCI] Bring clang-linker-wrapper closer to its upstream version
#16926
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Commit > ccd73ee > [LinkerWrapper] Remove in-house handling of LTO (llvm/llvm-project#113715) Removed a lot of dead code from `clang-linker-wrapper`. However, it has been preserved in our fork by: > d7c2a0f > Merge from 'main' to 'sycl-web' (69 commits) Due to the way we have handled a divergence from the upstream it isn't really clear if something is our code or not, so it wasn't that hard to make a conflict resolution mistake. This commit removes all that dead code to make it closer to the upstream.
We seem to have put all the community code into a separate branch which made it hard to maintain (i.e. keep in sync with the upstream). This commit outlines some common parts of downstream and upstream code to simplify feature maintanence (by making the code closer to the upstream overall).
AlexeySachkov
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
sarnex
approved these changes
Feb 7, 2025
AlexeySachkov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
bader
reviewed
Feb 7, 2025
|
|
||
| // Link the remaining device files using the device linker. | ||
| Expected<StringRef> OutputOrErr = | ||
| HasSYCLOffloadKind ? sycl::linkDevice(InputFiles, LinkerArgs) |
Contributor
There was a problem hiding this comment.
That's is wrong. We need to call both of these functions depending on the HasSYCLOffloadKind and HasNoSYCLOffloadKind. These two variables are independent of each other!
Contributor
There was a problem hiding this comment.
Ah..sorry. I missed this on my 1-1 with Alexey Sachkov. Yes. There is a possibility that both HasSYCLOffloadKind and HasNonSYCLOffloadKind are true. So, there is a possibility that both these functions need to run
Contributor
There was a problem hiding this comment.
And we are also missing linkBitcodeFiles call here. Are we not?
Contributor
Author
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR re-introduces a couple of commits which seems to be accidentally reverted during some merge conflict resolutions.
It also serves as an alternative version of #16879 with the intent to keep more of the community code and stop doing variables shadowing in our customizations