Skip to content

[SYCL][NewOffloadModel] Support -fno-sycl-rdc at compile step (-c) - #22833

Open
srividya-sundaram wants to merge 16 commits into
intel:syclfrom
srividya-sundaram:sycl-no-rdc-compile-step
Open

[SYCL][NewOffloadModel] Support -fno-sycl-rdc at compile step (-c)#22833
srividya-sundaram wants to merge 16 commits into
intel:syclfrom
srividya-sundaram:sycl-no-rdc-compile-step

Conversation

@srividya-sundaram

@srividya-sundaram srividya-sundaram commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds -fno-sycl-rdc -c support in the new offload model, matching old model ergonomics. Previously -fno-sycl-rdc was only meaningful at link time in the new model.

Each TU's device code is finalized at compile time via a per-TU clang-linker-wrapper --sycl-device-link --no-sycl-rdc invocation, producing a self-contained device image embedded into the host object via a new -fsycl-include-target-binary CC1 option.

Depends on #22832.

Key changes

  • clang/include/clang/Options/Options.td: New -fsycl-include-target-binary CC1 option (analog of -fcuda-include-gpubinary)
  • clang/include/clang/Basic/CodeGenOptions.h: New SYCLTargetBinaryFileName field to marshal the option
  • clang/lib/Driver/Driver.cpp: Insert per-TU LinkerWrapperJobAction in BuildOffloadingActions when -fno-sycl-rdc -c is used
  • clang/lib/Driver/ToolChains/Clang.cpp: Route finalized image to -fsycl-include-target-binary instead of -fembed-offload-object; use isDeviceOffloading(OFK_SYCL) to correctly scope --sycl-device-link to per-TU invocations only
  • clang/lib/CodeGen/CodeGenModule.cpp: Read finalized image and call wrapSYCLBinaries to embed and register at compile time
  • sycl/doc/design/OffloadDesign.md: Updated to reflect compile-step support is now implemented
  • sycl/test-e2e/AOT/early_aot.cpp: Updated comment to reflect both compile and link steps are supported
  • clang/test/Driver/sycl-no-rdc-compile-step.cpp: New driver test verifying per-TU pipeline shape, RDC regression, and link-step regression
  • clang/test/Driver/sycl-no-rdc-new-driver.cpp: Updated pipeline shape for 2-TU case; removed stale CHK-COMPILE-STEP-ERROR test

Implements compile-step support tracked in CMPLRLLVM-51875.

Test plan

  • clang/test/Driver/sycl-no-rdc-compile-step.cpp — per-TU pipeline shape, RDC regression, link-step regression
  • clang/test/Driver/sycl-no-rdc-new-driver.cpp — updated pipeline shape for 2-TU case

🤖 Generated with Claude Code

maksimsab and others added 6 commits July 30, 2026 13:44
Fix --no-offload-old-driver typo to --no-offload-new-driver in the
old offload model usage example. Add a note that compile-step support
for -fno-sycl-rdc (matching the old model's -c usage pattern) will
be implemented in a follow-up patch via clang-linker-wrapper
--sycl-device-link --no-sycl-rdc per translation unit.
Adds support for specifying -fno-sycl-rdc at the compile step in the new
offload model, matching the old offload model's usage pattern. Previously,
-fno-sycl-rdc was only meaningful at the link step in the new model.

Design (mirrors CUDA/HIP -fno-gpu-rdc per-TU finalize model):
- BuildOffloadingActions: when -fno-sycl-rdc is passed at compile time,
  insert a per-TU LinkerWrapperJobAction (OffloadPackager +
  clang-linker-wrapper --sycl-device-link --no-sycl-rdc) to finalize the
  TU's device code immediately, producing a self-contained device image.
- Clang::ConstructJob: route the finalized image to the new
  -fsycl-include-target-binary CC1 option instead of -fembed-offload-object,
  so the host CodeGen embeds and registers the image at compile time rather
  than deferring it to link time.
- CodeGenModule::Release: when -fsycl-include-target-binary is set on the
  host cc1, read the device image file and call wrapSYCLBinaries to embed
  the binary and emit __sycl_register_lib/__sycl_unregister_lib ctors into
  the host module. The final clang-linker-wrapper invocation then finds no
  SYCL device input and performs a plain host link.
- Downstream uses clang-linker-wrapper --sycl-device-link instead of the
  upstream's clang-sycl-linker, as all Intel SYCL pipeline logic resides in
  clang-linker-wrapper.

New files/options:
- -fsycl-include-target-binary <file>: new CC1 option (analog of
  -fcuda-include-gpubinary) marshalled into CodeGenOpts.SYCLTargetBinaryFileName
- clang/test/Driver/sycl-no-rdc-compile-step.cpp: driver tests verifying
  per-TU pipeline shape, RDC default regression, and link-step regression.

Implements compile-step support tracked in CMPLRLLVM-51875.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <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

Adds compile-step (-c) support for -fno-sycl-rdc in the new SYCL offload model by finalizing each TU’s device image during compilation (via a per-TU clang-linker-wrapper --sycl-device-link --no-sycl-rdc) and embedding/registering the finalized image into the host object using a new cc1 option.

Changes:

  • Teach the driver to insert a per-TU LinkerWrapperJobAction for SYCL NoRDC compilation and route its output to the host compile.
  • Introduce -fsycl-include-target-binary and plumb it through driver → CodeGen to embed/register the finalized SYCL image at compile time.
  • Add/update driver + e2e tests and update design documentation around NoRDC usage patterns.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
sycl/test-e2e/AOT/early_aot.cpp Adjust RUN lines for old/new model flag placement; adds explanatory note.
sycl/doc/design/OffloadDesign.md Document NoRDC behavior and usage scenarios for old/new offload models.
sycl/doc/design/NonRelocatableDeviceCode.md Clarify document scope (old offload model NoRDC design).
clang/test/Driver/sycl-no-rdc-new-driver.cpp New driver test for -fno-sycl-rdc propagation and pipeline shape.
clang/test/Driver/sycl-no-rdc-compile-step.cpp New driver test covering compile-step NoRDC finalization + cc1 plumbing.
clang/lib/Driver/ToolChains/Clang.cpp Pass finalized SYCL image to host cc1; propagate NoRDC to linker wrapper and add per-TU device-link mode logic.
clang/lib/Driver/Driver.cpp Insert per-TU linker-wrapper finalizer action for SYCL NoRDC during compilation.
clang/lib/CodeGen/CodeGenModule.cpp Read finalized SYCL image and call wrapSYCLBinaries to embed/register at compile time.
clang/include/clang/Options/Options.td Add new -fsycl-include-target-binary option (marshalled to CodeGenOpts).
clang/include/clang/Basic/CodeGenOptions.h Add SYCLTargetBinaryFileName storage for the new cc1 option.

Comment thread clang/lib/CodeGen/CodeGenModule.cpp Outdated
Comment thread clang/lib/Driver/ToolChains/Clang.cpp Outdated
Comment thread clang/lib/CodeGen/CodeGenModule.cpp Outdated
Comment thread sycl/doc/design/OffloadDesign.md Outdated
Comment thread sycl/test-e2e/AOT/early_aot.cpp Outdated

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

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

@srividya-sundaram

Copy link
Copy Markdown
Contributor Author

SYCL Pre commit failure is unrelated to the changes in this PR.

@srividya-sundaram

Copy link
Copy Markdown
Contributor Author

Please note that this PR is a follow-up to #22832 and currently includes its changes as a base. Once #22832 lands, this branch will be rebased on top of it so the diff only shows the compile-step additions.

@srividya-sundaram
srividya-sundaram marked this pull request as ready for review July 31, 2026 18:59
@srividya-sundaram
srividya-sundaram requested review from a team as code owners July 31, 2026 18:59
HelpText<"Incorporate CUDA device-side binary into host object file.">,
MarshallingInfoString<CodeGenOpts<"CudaGpuBinaryFileName">>;
def fsycl_include_target_binary : Separate<["-"], "fsycl-include-target-binary">,
HelpText<"Incorporate SYCL device-side binary into host object file.">,

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.

kind of a nit but i since the device binary will contain information on the offloading model and triple used (as it should be inside the OffloadBinary wrapper), i don't see any need for per-offload-model options, but since CUDA already did it, i guess we only can address this feedback when we implement an equivalent version of this change upstream, where we can unify them

@srividya-sundaram srividya-sundaram Jul 31, 2026

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.

i guess we only can address this feedback when we implement an equivalent version of this change upstream, where we can unify them

Agree.

This option was introduced based on the SYCL no-RDC design doc from @YuriPlyakhin : https://github.com/YuriPlyakhin/llvm-project/blob/6553d362c5e84114a30eff3a79cad70432fff3ef/sycl-no-rdc-plan.md#3-per-tu-device-finalize--host-compile-inclusion-cudahip-mirror

We can unify with the CUDA/HIP option when implementing the equivalent upstream change.

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.

Would it perhaps make sense to name the new option -foffload-include-binary or similar in anticipation of future unification?

Comment thread clang/lib/CodeGen/CodeGenModule.cpp Outdated
Comment thread clang/lib/CodeGen/CodeGenModule.cpp Outdated
CmdArgs.push_back(CudaDeviceInput->getFilename());
} else if (!HostOffloadingInputs.empty()) {
if ((IsCuda || IsHIP) && !IsRDCMode) {
// CUDA/HIP no-RDC: device image finalized per-TU, embed at compile time.

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.

since HIP already goes through this path, can we reuse the same -fcuda-include-gpubinary option so instead we could just change the if to if((IsCuda || isHIP || isSYCL) && !IsRDCMode)? this is kind of related to my comment before about the option

Comment thread clang/lib/Driver/ToolChains/Clang.cpp Outdated
Comment thread clang/test/Driver/sycl-no-rdc-compile-step.cpp Outdated
Comment thread sycl/doc/design/OffloadDesign.md Outdated
Comment thread sycl/doc/design/OffloadDesign.md
Comment thread sycl/test-e2e/AOT/early_aot.cpp Outdated
Comment thread sycl/test-e2e/AOT/early_aot.cpp Outdated
sarnex pushed a commit that referenced this pull request Jul 31, 2026
…Offload Model (#22832)

## Summary

This patch adds `-fno-sycl-rdc` support to the new SYCL offload model in
the Clang driver. It is based on the work by @maksimsab in #21973, with
review fixes applied.

### What this patch does

By default (`-fsycl-rdc`), all device code across translation units is
linked together into one module at link time before post-link
processing. With `-fno-sycl-rdc`, each object file's device code is
processed independently — skipping the `llvm-link` step — which can
significantly reduce peak memory and compile time.

**Changes:**
- `clang/lib/Driver/ToolChains/Clang.cpp`: In
`LinkerWrapper::ConstructJob`, propagate `--no-sycl-rdc` to
`clang-linker-wrapper` when `-fno-sycl-rdc` is passed at link time.
- `sycl/doc/design/OffloadDesign.md`: Document NoRDC mode for the new
offload model.
- `sycl/doc/design/NonRelocatableDeviceCode.md`: Add a note clarifying
that document covers the old offload model only.
- `sycl/test-e2e/AOT/early_aot.cpp`: Un-XFAIL the test for the new
offload model; adjust RUN lines to pass `-fno-sycl-rdc` at the correct
step per model.
- `clang/test/Driver/sycl-no-rdc-new-driver.cpp`: New driver test
verifying `--no-sycl-rdc` is propagated to `clang-linker-wrapper`
correctly.

### Key behavioral difference from the old offload model

| Model | Where to pass `-fno-sycl-rdc` |
|---|---|
| Old (`--no-offload-new-driver`) | At each **compile** step (`-c`) |
| New (`--offload-new-driver`) | At the **link** step |

This inversion exists because in the new model all SYCL offload
processing (post-link, AOT, wrapping) lives in `clang-linker-wrapper` at
link time.

### Follow-up work

Supporting `-fno-sycl-rdc -c` in the new model (matching the old model's
compile-step ergonomics) will be addressed in a follow-up patch. The
plan is to invoke `clang-linker-wrapper --sycl-device-link
--no-sycl-rdc` per translation unit at compile time, embedding the
finalized device image directly into the host object.

Supporting -fno-sycl-rdc -c in the new model (matching old model
compile-step ergonomics) is addressed in #22833.

Fixes: CMPLRLLVM-51875

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Sabianin, Maksim <maksim.sabianin@intel.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
/// action and routes the result to -fsycl-include-target-binary on the host
/// cc1, instead of deferring device processing to link time.

// RUN: touch %t.cpp

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 didn't catch this in the first RDC PR, but why do we need to do this? We should be able just to use %s in the tests.

srividya-sundaram and others added 5 commits July 31, 2026 16:36
Move embedSYCLNoRDCBinary out of CodeGenModule::Release() into a free
function in CGSYCLRuntime.cpp to keep SYCL-specific logic out of common
code. CodeGenModule::Release() retains a single call site.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

6 participants