Skip to content

[SYCL] Add option forwarding from clang-linker-wrapper tool to clang-sycl-linker #21958

Open
srividya-sundaram wants to merge 1 commit intointel:syclfrom
srividya-sundaram:clw-to-csl
Open

[SYCL] Add option forwarding from clang-linker-wrapper tool to clang-sycl-linker #21958
srividya-sundaram wants to merge 1 commit intointel:syclfrom
srividya-sundaram:clw-to-csl

Conversation

@srividya-sundaram
Copy link
Copy Markdown
Contributor

@srividya-sundaram srividya-sundaram commented May 7, 2026

This is the first patch in a series that moves SYCL device code linking logic from clang-linker-wrapper into the clang-sycl-linker tool.
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-linker invocations will be added in subsequent PRs.

Added infrastructure from PR 21692 + tests for option forwarding.

@srividya-sundaram srividya-sundaram marked this pull request as ready for review May 8, 2026 00:27
@srividya-sundaram srividya-sundaram requested review from a team as code owners May 8, 2026 00:27
return createStringError("Could not link IR");
}

// Get all SYCL device library files, if any.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So,

  1. Submit PR21672 upstream.
  2. Submit --device-libs and --library-path cleanup 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?

Copy link
Copy Markdown
Contributor

@YuriPlyakhin YuriPlyakhin May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@maksimsab
Copy link
Copy Markdown
Contributor

More general comment.
I see that your PR targets 2 independent goals:

  • Add option forwarding
  • Remove deprecated device library options

In cases like that the suggested approach is:

The patch should:
* be an isolated change. Independent changes should be submitted as separate patches as this makes reviewing easier.

https://llvm.org/docs/Contributing.html#how-to-submit-a-patch

Please, use this approach in your patches.

Copy link
Copy Markdown
Contributor

@YuriPlyakhin YuriPlyakhin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you please update title?

@srividya-sundaram srividya-sundaram changed the title [SYCL] Add option forwarding and remove deprecated device library options [SYCL] Add option forwarding from clang linker wrapper to clang sycl linker May 8, 2026
@srividya-sundaram srividya-sundaram changed the title [SYCL] Add option forwarding from clang linker wrapper to clang sycl linker [SYCL] Add option forwarding from clang-linker-wrapper tool to clang-sycl-linker May 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants