[SYCL] Add option forwarding from clang-linker-wrapper tool to clang-sycl-linker #21958
[SYCL] Add option forwarding from clang-linker-wrapper tool to clang-sycl-linker #21958srividya-sundaram wants to merge 1 commit intointel:syclfrom
Conversation
| return createStringError("Could not link IR"); | ||
| } | ||
|
|
||
| // Get all SYCL device library files, if any. |
There was a problem hiding this comment.
I think this clean up should be submitted upstream. But before it is submitted upstream, I think we need to submit to upstream importing of device libraries in code-gen stage, similar to what we did in downstream.
Otherwise, functionality between community clang-sycl-linker and downstream clang-sycl-linker diverges a lot and it will be incrementally more and more difficult to maintain 2 variants of code.
There was a problem hiding this comment.
So,
- Submit PR21672 upstream.
- Submit
--device-libsand--library-pathcleanup upstream.
Is this correct understanding?
If yes, I can work on # 1 and # 2 submissions to upstream, and we can split this PR to have just the option forwarding + test changes and add the device libs cleanup after the upstream patches land.
WDYT?
There was a problem hiding this comment.
Yes, that sounds correct to me. I don't know how 1. would look like in upstream (unlikely can be submitted as-is), since it seems there might be some functionality missing yet or code is different, but it would be the best way to move forward in my opinion.
|
More general comment.
In cases like that the suggested approach is: https://llvm.org/docs/Contributing.html#how-to-submit-a-patch Please, use this approach in your patches. |
8d6725d to
20f67e1
Compare
YuriPlyakhin
left a comment
There was a problem hiding this comment.
could you please update title?
This is the first patch in a series that moves SYCL device code linking logic from
clang-linker-wrapperinto theclang-sycl-linkertool.I’m continuing the work from where Yixing left off.
At the moment, the tests only verify that the options are forwarded to
clang-linker-wrapper.Tests validating
clang-sycl-linkerinvocations will be added in subsequent PRs.Added infrastructure from PR 21692 + tests for option forwarding.