[SYCL] Add support for passing multiple archs in command line by bviyer · Pull Request #22945 · intel/llvm

bviyer · 2026-08-14T20:40:05Z

The clang-linker-wrapper --device-compiler=/--device-linker= channel did not distinguish between architectures sharing the same triple, so with-fsycl-targets=spir64_gen,intel_gpu_skl plus per-target -Xsycl-target-backend, all options were emitted under a single spir64_gen-unknown-unknown entry and per-arch tokens leaked across ocloc invocations (e.g. skl's options ended up on the pvc call and vice versa). The driver now emits one --device-compiler/--device-linker per (triple, arch) with tokens joined into a single value; gen entries carry a leading "-device " that the wrapper uses to route each value to the matching ocloc call, while values without "-device" (or from non-gen triples) still apply to every arch of the triple. This feature affects the new-offload-model only.

bader · 2026-08-17T20:24:30Z

The clang-linker-wrapper --device-compiler=/--device-linker= channel did not distinguish between architectures sharing the same triple, so with-fsycl-targets=spir64_gen,intel_gpu_skl plus per-target -Xsycl-target-backend, all options were emitted under a single spir64_gen-unknown-unknown entry and per-arch tokens leaked across ocloc invocations (e.g. skl's options ended up on the pvc call and vice versa).

I think we need to fix this design issue to enable support for "multiple archs" i.e. we must use a dedicated key for each -fsycl-target= value.

The driver now emits one --device-compiler/--device-linker per (triple, arch) with tokens joined into a single value; gen entries carry a leading "-device " that the wrapper uses to route each value to the matching ocloc call, while values without "-device" (or from non-gen triples) still apply to every arch of the triple. This feature affects the new-offload-model only.

Joining all options into a single value to reparse them again in clang-link-wrapper tool requires implementing non-trivial logic which is a source of bugs. As Mike noted in his comment, dd9abc1 replaces this approach with simplified logic to fix one of such bugs. I suggest we don't bring it back.

bviyer · 2026-08-19T20:48:58Z

The clang-linker-wrapper --device-compiler=/--device-linker= channel did not distinguish between architectures sharing the same triple, so with-fsycl-targets=spir64_gen,intel_gpu_skl plus per-target -Xsycl-target-backend, all options were emitted under a single spir64_gen-unknown-unknown entry and per-arch tokens leaked across ocloc invocations (e.g. skl's options ended up on the pvc call and vice versa).

I think we need to fix this design issue to enable support for "multiple archs" i.e. we must use a dedicated key for each -fsycl-target= value.

The driver now emits one --device-compiler/--device-linker per (triple, arch) with tokens joined into a single value; gen entries carry a leading "-device " that the wrapper uses to route each value to the matching ocloc call, while values without "-device" (or from non-gen triples) still apply to every arch of the triple. This feature affects the new-offload-model only.

Joining all options into a single value to reparse them again in clang-link-wrapper tool requires implementing non-trivial logic which is a source of bugs. As Mike noted in his comment, dd9abc1 replaces this approach with simplified logic to fix one of such bugs. I suggest we don't bring it back.

I have put it back. I also added a e2e test.

mdtoguchi · 2026-08-20T22:06:38Z

+// WRAPPER_OPTIONS_MULTI_GEN-SAME: "--device-compiler=sycl:spir64_gen-unknown-unknown/pvc=-extraopt_pvc"
+// WRAPPER_OPTIONS_MULTI_GEN-SAME: "--device-compiler=sycl:spir64_gen-unknown-unknown/skl=-extraopt_skl"
+// WRAPPER_OPTIONS_MULTI_GEN-NOT: "--device-compiler=sycl:spir64_gen-unknown-unknown/pvc=-extraopt_skl"
+// WRAPPER_OPTIONS_MULTI_GEN-NOT: "--device-compiler=sycl:spir64_gen-unknown-unknown/skl=-extraopt_pvc"


Can a test be added that verifies the mixing of intel_gpu* and spir64_gen with -device usage where the target is the same? e.g.: -fsycl --offload-new-driver -fsycl-targets=spir64_gen,intel_gpu_skl -Xsycl-target-backend=spir64_gen "-device skl -options extraopt_skl1" -Xsycl-target-backend=intel_gpu_skl "-options -extraopt_skl2"?

The test was added, but it only asserts the driver's argv, not the ocloc argv. Could you please add ocloc checks as well to make sure merging of options works as expected?

mdtoguchi

OK by me

sys-ce-bb · 2026-08-24T23:06:29Z

@intel/llvm-gatekeepers please consider merging

Co-authored-by: Yury Plyakhin <yury.plyakhin@intel.com>

YuriPlyakhin

Please, update sycl/doc/design/OffloadDesign.md in this PR as well.

It documents --device-compiler=[<kind>:][<triple>=]<value>

I think we should document:

[<kind>:][<triple>[/<arch>]=]<value> format
the multi-arch example needs update
one token per occurrence
what happens with no /<arch> provided

YuriPlyakhin · 2026-08-27T14:45:57Z

                         Triple.getSubArch() == llvm::Triple::SPIRSubArch_gen;
      if (IsGenTriple) {
-        if (Device != GenDevice && !Device.empty())
+        if (!GenDevice.empty() && Device != GenDevice && !Device.empty())


It seems to change old offload model ocloc invocations.

Could you please check these examples, before and after your change - what does each call to ocloc receives?
clang++ -fsycl -fsycl-targets=intel_gpu_dg1 -Xsycl-target-backend=spir64_gen "-DFOO"
clang++ -fsycl -fsycl-targets=intel_gpu_dg1,spir64_gen,intel_gpu_skl -Xsycl-target-backend=spir64_gen "-device skl -DSKL" -Xsycl-target-backend=intel_gpu_skl "-DSKL2"

Please, look at CHECK_TOOLS_BEOPTS test case in clang/test/Driver/sycl-oneapi-gpu-intelgpu.cpp, I think it can be used to verify the change before/after. The checks there are loose, so they might not catch the change.

If old offloading model behavior changes, I think we need to decide, if it is what we want...

YuriPlyakhin · 2026-08-27T15:05:10Z

+// RUN:          -fsycl-targets=spir64_gen,intel_gpu_skl \
+// RUN:          -Xsycl-target-backend=spir64_gen "-device skl -options extraopt_skl1" \
+// RUN:          -Xsycl-target-backend=intel_gpu_skl "-options -extraopt_skl2" \


In the new model these two -fsycl-targets entries collapse to a single arch (skl), so both option groups reach one ocloc invocation, right?
In the old offload model, it seems we wanted different behavior, check clang/test/Driver/sycl-oneapi-gpu-intelgpu.cpp, specifically:

/// Check that ocloc backend option settings only occur for the expected /// toolchains when mixing spir64_gen and intel_gpu // RUN: %clangxx -fsycl -fsycl-targets=intel_gpu_dg1,spir64_gen,intel_gpu_skl \ // RUN: -Xsycl-target-backend=spir64_gen "-device skl -DSKL" \ // RUN: -Xsycl-target-backend=intel_gpu_dg1 "-DDG1" \ // RUN: -Xsycl-target-backend=intel_gpu_skl "-DSKL2" \ // RUN: --no-offloadlib -fno-sycl-instrument-device-code \ // RUN: -target x86_64-unknown-linux-gnu -### %s 2>&1 | \ // RUN: FileCheck %s --check-prefix=CHECK_TOOLS_BEOPTS // CHECK_TOOLS_BEOPTS: ocloc{{.*}} "-device" "dg1"{{.*}}"-DDG1" // CHECK_TOOLS_BEOPTS: ocloc{{.*}} "-device" "skl"{{.*}}"-DSKL" // CHECK_TOOLS_BEOPTS: ocloc{{.*}} "-device" "skl"{{.*}}"-DSKL2"

YuriPlyakhin · 2026-08-27T15:19:52Z

+          llvm::cl::TokenizeGNUCommandLine(Value, S, Tokens);
+          bool EmbDeviceNoMatch = false;
+          for (size_t I = 0; I + 1 < Tokens.size(); ++I) {
+            if (StringRef(Tokens[I]) == "-device") {


There are two -device scanners now, with semantics that disagree.

Please, check: Driver::getOffloadArchs (Driver.cpp:7925) — scans right-to-left, grabs the right most entry
This one — scans left-to-right, takes the left most entry.

So -Xsycl-target-backend=spir64_gen "-device pvc -device skl" gives Arch=skl from getOffloadArchs
but routes options against pvc here.

Is it possible to make a single helper?

What happens with the cases like -device pvc,bdw?

YuriPlyakhin · 2026-08-27T15:29:54Z

+          SmallVector<const char *, 8> Tokens;
+          llvm::BumpPtrAllocator Alloc;
+          llvm::StringSaver S(Alloc);
+          llvm::cl::TokenizeGNUCommandLine(Value, S, Tokens);


nit
This tokenizes A->getValue(1), and parseTargetOpts tokenizes the same string again a few lines below.
Consider refactoring, if practical.

YuriPlyakhin · 2026-08-27T15:33:49Z

+// WRAPPER_OPTIONS_MULTI_GEN-NOT: "--device-compiler=sycl:spir64_gen-unknown-unknown/pvc=-extraopt_skl"
+// WRAPPER_OPTIONS_MULTI_GEN-NOT: "--device-compiler=sycl:spir64_gen-unknown-unknown/skl=-extraopt_pvc"


Everything is on one clang-linker-wrapper line, so a cross-contaminated entry appearing earlier on that line would not be caught. Use --implicit-check-not=....

YuriPlyakhin · 2026-08-27T15:43:38Z

+//      (runtime check).
+
+// REQUIRES: ocloc, target-spir
+// REQUIRES: arch-intel_gpu_pvc


I think compile-time test should be enough for this PR. I don't see why we need to run it.

Could you please consider dropping %{run} and the PVC requirement REQUIRES: arch-intel_gpu_pvc (but keep REQUIRES: ocloc)?

I think we may also have some other lit parameter that specifies "build only" tests, maybe we can apply it as well.

We want this test to run each time, when the build of E2E tests is happening.

Also please, check if we have ocloc on the systems, which are used to build E2E tests.

Thanks!

YuriPlyakhin · 2026-08-27T15:46:10Z

+    // For spir64_gen the value is qualified with "/<arch>" and emitted per
+    // (triple, arch) to keep per-arch tokens from crossing across archs.


Suggested change

// For spir64_gen the value is qualified with "/<arch>" and emitted per

// (triple, arch) to keep per-arch tokens from crossing across archs.

// For spir64_gen the key is qualified with "/<arch>" and emitted per

// (triple, arch) to keep per-arch tokens from leaking between archs.

YuriPlyakhin · 2026-08-27T15:52:37Z

+/// /skl bucket rather than land under different keys or drop each other.
+// RUN: %clangxx --target=x86_64-unknown-linux-gnu -fsycl --offload-new-driver --sysroot=%S/Inputs/SYCL \
+// RUN:          -fsycl-targets=spir64_gen,intel_gpu_skl \
+// RUN:          -Xsycl-target-backend=spir64_gen "-device skl -options extraopt_skl1" \


nit, for consistency with the next line

Suggested change

// RUN: -Xsycl-target-backend=spir64_gen "-device skl -options extraopt_skl1" \

// RUN: -Xsycl-target-backend=spir64_gen "-device skl -options -extraopt_skl1" \

YuriPlyakhin

From description: The driver now emits one --device-compiler/--device-linker per (triple, arch) with tokens joined into a single value;

Is it still up-to-date statement?

YuriPlyakhin

Please, consider the following case (pseudo-code):

fsycltargets=spir64_gen,pvc,skl
backendoption=spir64_gen "-device skl -DFOO1"
backendoption=spir64_gen "=-DFOO4"
backendoption=pvc "=-device pvc -DFOO2"
backendoption=skl "=-DFOO3"

I believe, the old model would call oclocs the following way (please, check that):

ocloc -device skl -DFOO1 -DFOO4
ocloc -device pvc -device pvc -DFOO2
ocloc -device skl -DFOO3

With the current patch, I think we will get (please check that):

ocloc	"-device skl -DFOO1 -DFOO4 -DFOO3
ocloc	"-device pvc -device pvc -DFOO2 -DFOO4

It doesn't make sense to keep spir64_gen in the new offloading model, but change the semantics. If we keep it in new offloading model, it should work the same way as in old offloading model.

Thinking aloud:
What if we introduce our internal arch, something like "aot_generic", and use it for spir64_gen, without trying to parse -device option? So all options, passed to `spir64_gen" would just end up as is in the single ocloc call? Would that work and match what the old offloading model does?

YuriPlyakhin · 2026-08-27T16:44:24Z

+          // Raw spir64_gen entry: if the value embeds "-device X", route
+          // only to arch X. Absent -> shared, applies to every arch.


is this reproducing semantics of old offloading model?

YuriPlyakhin · 2026-08-27T16:51:03Z

+// CHECK-RAW-PVC: ocloc{{.*}} -device pvc {{.*}}-cl-mad-enable
+// CHECK-RAW-PVC-NOT: ocloc{{.*}} -device pvc {{.*}}-cl-unsafe-math-optimizations
+// CHECK-RAW-ACM: ocloc{{.*}} -device acm_g10 {{.*}}-cl-unsafe-math-optimizations
+// CHECK-RAW-ACM-NOT: ocloc{{.*}} -device acm_g10 {{.*}}-cl-mad-enable


consider implicit-check-not in this and other cases to avoid test false-passing, because of order of parameters in the same line.

YuriPlyakhin · 2026-08-27T16:51:49Z

+// CHECK-RAW-ACM: ocloc{{.*}} -device acm_g10 {{.*}}-cl-unsafe-math-optimizations
+// CHECK-RAW-ACM-NOT: ocloc{{.*}} -device acm_g10 {{.*}}-cl-mad-enable
+
+#include <sycl/detail/core.hpp>


if we end up in build-only test, I think, the source code can be simplified.

sys-ce-bb · 2026-08-28T01:42:40Z

@intel/llvm-gatekeepers please consider merging

sys-ce-bb · 2026-08-31T02:15:51Z

@intel/llvm-gatekeepers please consider merging

bviyer · 2026-08-31T22:55:44Z

Please, consider the following case (pseudo-code):
fsycltargets=spir64_gen,pvc,skl
backendoption=spir64_gen "-device skl -DFOO1"
backendoption=spir64_gen "=-DFOO4"
backendoption=pvc "=-device pvc -DFOO2"
backendoption=skl "=-DFOO3"
I believe, the old model would call oclocs the following way (please, check that):
ocloc -device skl -DFOO1 -DFOO4
ocloc -device pvc -device pvc -DFOO2
ocloc -device skl -DFOO3
With the current patch, I think we will get (please check that):
ocloc	"-device skl -DFOO1 -DFOO4 -DFOO3
ocloc	"-device pvc -device pvc -DFOO2 -DFOO4
It doesn't make sense to keep spir64_gen in the new offloading model, but change the semantics. If we keep it in new offloading model, it should work the same way as in old offloading model.

Thinking aloud: What if we introduce our internal arch, something like "aot_generic", and use it for spir64_gen, without trying to parse -device option? So all options, passed to `spir64_gen" would just end up as is in the single ocloc call? Would that work and match what the old offloading model does?

I am getting hte following output:

$ ./bin/clang++ -fsycl -fsycl-targets=spir64_gen,intel_gpu_pvc,intel_gpu_skl   -Xsycl-target-backend=spir64_gen "-device skl -DFOO1"   -Xsycl-target-backend=spir64_gen "-DF004"   -Xsycl-target-backend=intel_gpu_skl "-DF003" -Xsycl-target-backend=intel_gpu_pvc "-device pvc -DF002"
 --sysroot="$SYSROOT" -target x86_64-unknown-linux-gnu tst.cpp -### 2>&1 | grep -
oE '[^"]*ocloc"[^<]*'
<snip>/ocloc" "-output" "/tmp/tst-679107.out" "-file" "/tmp/tst-bafa8c.txt" "-output_no_suffix" "-spirv_input" "-device" "skl" "-DFOO1" "-DF004" "-DF003" "-device" "pvc" "-DF002"
<snip>/ocloc" "-output" "/tmp/tst-pvc-c44c0b.out" "-file" "/tmp/tst-pvc-7baf0a.txt" "-output_no_suffix" "-spirv_input" "-device" "pvc" "-device_options" "pvc" "-ze-intel-enable-auto-large-GRF-mode" "-device" "pvc" "-DF002"
<snip>/ocloc" "-output" "/tmp/tst-skl-0c6e62.out" "-file" "/tmp/tst-skl-8f82b8.txt" "-output_no_suffix" "-spirv_input" "-device" "skl" "-DF003"

Without my change (or the sycl branch):

$ git status
On branch sycl
Your branch is up to date with 'origin/sycl'.

nothing to commit, working tree clean
$ ./bin/clang++ -fsycl -fsycl-targets=spir64_gen,intel_gpu_pvc,intel_gpu_skl   -Xsycl-target-backend=spir64_gen "-device skl -DFOO1"   -Xsycl-target-backend=spir64_gen "-DF004"   -Xsycl-target-backend=intel_gpu_skl "-DF003" -Xsycl-target-backend=intel_gpu_pvc "-device pvc -DF002"   --sysroot="$SYSROOT" -target x86_64-unknown-linux-gnu tst.cpp  -### 2>&1  | grep -oE '[^"]*ocloc"[^<]*'
<snip>/ocloc" "-output" "/tmp/tst-1ff793.out" "-file" "/tmp/tst-4a8c7e.txt" "-output_no_suffix" "-spirv_input" "-device" "skl" "-DFOO1" "-DF004" "-DF003" "-device" "pvc" "-DF002"
<snip>/ocloc" "-output" "/tmp/tst-pvc-788414.out" "-file" "/tmp/tst-pvc-65c933.txt" "-output_no_suffix" "-spirv_input" "-device" "pvc" "-device_options" "pvc" "-ze-intel-enable-auto-large-GRF-mode" "-device" "pvc" "-DF002"
<snip>/ocloc" "-output" "/tmp/tst-skl-33080c.out" "-file" "/tmp/tst-skl-40f417.txt" "-output_no_suffix" "-spirv_input" "-device" "skl" "-DF003"

bviyer requested review from a team as code owners August 14, 2026 20:40

bviyer changed the title ~~Added changes to support multiple archs in command line~~ [SYCL] Added changes to support multiple archs in command line Aug 14, 2026

bader reviewed Aug 14, 2026

View reviewed changes

Comment thread clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp Outdated

mdtoguchi reviewed Aug 14, 2026

View reviewed changes

Comment thread clang/lib/Driver/ToolChains/Clang.cpp Outdated

bviyer requested review from YuriPlyakhin, bader, mdtoguchi and sarnex August 14, 2026 21:22

mdtoguchi reviewed Aug 20, 2026

View reviewed changes

Comment thread clang/lib/Driver/ToolChains/Clang.cpp Outdated

Added changes to support multiple archs in command line

d5cc96a

bviyer force-pushed the bviyer-support-passing-multiple-args branch from 196d3a8 to d5cc96a Compare August 22, 2026 00:49

bviyer requested a review from mdtoguchi August 22, 2026 00:50

mdtoguchi approved these changes Aug 24, 2026

View reviewed changes

bader reviewed Aug 24, 2026

View reviewed changes

Comment thread clang/lib/Driver/ToolChains/SYCL.cpp Outdated

YuriPlyakhin reviewed Aug 25, 2026

View reviewed changes

Comment thread clang/lib/Driver/ToolChains/Clang.cpp Outdated

Update clang/lib/Driver/ToolChains/Clang.cpp

f008ebc

Co-authored-by: Yury Plyakhin <yury.plyakhin@intel.com>

bviyer requested review from YuriPlyakhin, bader and mdtoguchi August 26, 2026 21:53

mdtoguchi approved these changes Aug 26, 2026

View reviewed changes

bviyer changed the title ~~[SYCL] Added changes to support multiple archs in command line~~ [SYCL] Add support for passing multiple archs in command line Aug 26, 2026

YuriPlyakhin reviewed Aug 27, 2026

View reviewed changes

YuriPlyakhin added the new-offload-model Enables testing with NewOffloadModel. label Aug 27, 2026

YuriPlyakhin reviewed Aug 27, 2026

View reviewed changes

bviyer added 2 commits August 31, 2026 12:31

Stashing changes

dd4ce57

Stashing my changes

a768458

Fixed more things

16cb0e5

bviyer requested a review from a team as a code owner September 1, 2026 22:25

		// WRAPPER_OPTIONS_MULTI_GEN-NOT: "--device-compiler=sycl:spir64_gen-unknown-unknown/pvc=-extraopt_skl"
		// WRAPPER_OPTIONS_MULTI_GEN-NOT: "--device-compiler=sycl:spir64_gen-unknown-unknown/skl=-extraopt_pvc"

		// For spir64_gen the value is qualified with "/<arch>" and emitted per
		// (triple, arch) to keep per-arch tokens from crossing across archs.

	// RUN: -Xsycl-target-backend=spir64_gen "-device skl -options extraopt_skl1" \
	// RUN: -Xsycl-target-backend=spir64_gen "-device skl -options -extraopt_skl1" \

		// Raw spir64_gen entry: if the value embeds "-device X", route
		// only to arch X. Absent -> shared, applies to every arch.

Uh oh!

Conversation

bviyer commented Aug 14, 2026

Uh oh!

Uh oh!

Uh oh!

bader commented Aug 17, 2026

Uh oh!

bviyer commented Aug 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mdtoguchi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sys-ce-bb commented Aug 24, 2026

Uh oh!

Uh oh!

YuriPlyakhin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YuriPlyakhin left a comment

Choose a reason for hiding this comment

Uh oh!

YuriPlyakhin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sys-ce-bb commented Aug 28, 2026

Uh oh!

sys-ce-bb commented Aug 31, 2026

Uh oh!

bviyer commented Aug 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

YuriPlyakhin left a comment •

edited

Loading