Skip to content

[SYCL][Driver] Forward SYCL tool options as individual tokens#22693

Open
sarnex wants to merge 4 commits into
intel:syclfrom
sarnex:forward
Open

[SYCL][Driver] Forward SYCL tool options as individual tokens#22693
sarnex wants to merge 4 commits into
intel:syclfrom
sarnex:forward

Conversation

@sarnex

@sarnex sarnex commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Extend #22645 to apply for -Xdevice-post-link, -Xspirv-translator and -Xspirv-to-ir.

CMPLRLLVM-77053

Signed-off-by: Nick Sarnie <nick.sarnie@intel.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

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.

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR updates SYCL-related option forwarding so that tool options are passed as individual tokens (via repeated --*-options= occurrences) rather than as a single space-delimited string that is re-split later in clang-linker-wrapper.

Changes:

  • Forward -Xdevice-post-link, -Xspirv-translator, and -Xspirv-to-ir-wrapper options as individual tokens (no whitespace re-splitting in the linker-wrapper).
  • Update LinkerWrapper::ConstructJob to emit repeated --sycl-post-link-options= / --llvm-spirv-options= / --spirv-to-ir-wrapper-options= arguments per token.
  • Adjust and extend driver tests to validate multi-token forwarding behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp Stops splitting forwarded option strings on spaces; expects tokens to arrive already separated.
clang/lib/Driver/ToolChains/Clang.cpp Emits one wrapper option occurrence per token instead of a single joined string.
clang/test/Driver/sycl-spirv-to-ir-opt.cpp Adds coverage for multi-token -Xspirv-to-ir-wrapper forwarding.
clang/test/Driver/sycl-spirv-opt.cpp Adds coverage for multi-token -Xspirv-translator forwarding.
clang/test/Driver/sycl-device-post-link-opt.cpp Adds coverage for multi-token -Xdevice-post-link forwarding.
clang/test/Driver/sycl-offload-new-driver.cpp Updates checks to reflect per-token --sycl-post-link-options= emission.
clang/test/Driver/sycl-linker-wrapper-image.cpp Updates direct wrapper invocation to pass repeated -sycl-post-link-options tokens.
clang/test/Driver/sycl-spirv-metadata.cpp Updates check to match new per-token --llvm-spirv-options= behavior.

Comment thread clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
Comment thread clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
Comment thread clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
Comment thread clang/test/Driver/sycl-offload-new-driver.cpp Outdated

@bader bader left a comment

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.

LGTM with one suggestion for test improvement (which applies to my PR as well :)).

Comment thread clang/test/Driver/sycl-spirv-to-ir-opt.cpp
sarnex added 2 commits July 21, 2026 07:34
Signed-off-by: Nick Sarnie <nick.sarnie@intel.com>
@sarnex sarnex added the new-offload-model Enables testing with NewOffloadModel. label Jul 21, 2026

@mdtoguchi mdtoguchi left a comment

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.

LGTM - Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new-offload-model Enables testing with NewOffloadModel.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants