[NewOffloadModel] Refactor device compiler and linker option parsing in ClangLinkerWrapper by YixingZhang007 · Pull Request #21055 · intel/llvm

YixingZhang007 · 2026-01-14T17:31:40Z

This patch refactors the parsing of device compiler and linker options in clang-linker-wrapper.

The specific changes are listed below:

Merged appendSYCLDeviceOptionsAtLinkTime() directly into getLinkerArgs() in ClangLinkerWrapper.cpp, so the ArgList returned by getLinkerArgs() includes both linker_arg_EQ and compiler_arg_EQ options, which are the filtered options of OPT_device_linker_args_EQ and OPT_device_compiler_args_EQ.
OPT_compiler_arg_EQ and OPT_linker_arg_EQ are now storing both the options passed to the backend compiler and the options passed to the Clang command for linking.

With this patch, the SYCL E2E test KernelAndProgram/kernel-bundle-merge-options-env.cpp pass with the new offloading mode.

YixingZhang007 · 2026-02-10T23:01:34Z

I haven’t seen any issues with removing the -linker-arg= and -compiler-arg= options passed to clang-linker-wrapper, and I don’t see any tests that rely on them either. However, please let me know if I’ve missed anything or if you have any other suggestions. Thank you!

YuriPlyakhin · 2026-02-11T00:30:00Z

I haven’t seen any issues with removing the -linker-arg= and -compiler-arg= options passed to clang-linker-wrapper, and I don’t see any tests that rely on them either. However, please let me know if I’ve missed anything or if you have any other suggestions. Thank you!

I guess it is because our variant of new offloading model is not using them, but most likely community variant of the code (for other programming models at least) will likely fail, after this change.

YixingZhang007 · 2026-02-18T21:32:45Z

I don’t think the current CI failure is related to the change in this PR, as our change only affects tests with the new offloading model.

sarnex · 2026-02-19T19:35:40Z

+          continue;
+        Arg = Arg.substr(ColonPos + 1);
+      }
+      size_t EqPos = Arg.find('=');


Can you add a comment giving an example value of Arg and then explain how we process it? It seems we are looking both for a : and for =. I want to understand the processing we are doing here a bit better

For sure! I’ve added the following comment before the lambda function to explain the --device-compiler and --device-linker arguments passed to the clang-linker-wrapper.

// The device_linker_args and device_compiler_args options accept values // in the form [<kind>:][<triple>=]<value>. // An example of passing such an option to clang-linker-wrapper is: // --device-compiler=spir64_gen-unknown-unknown=opt_val.

sarnex · 2026-02-19T19:40:37Z

+  // Filter the SYCL device compiler and linker options by target triple and
+  // offload kind.
+  const StringRef TripleStr = DAL.getLastArgValue(OPT_triple_EQ);
+  auto processDeviceArgs = [&](unsigned DeviceOptID, unsigned OptID) {


Can we give these arguments some more descriptive names?

Thanks for the suggestion! I have changed it to be

auto ProcessDeviceArgs = [&](unsigned DeviceArgsOptionID, unsigned ForwardedOptionID) {

sarnex · 2026-02-19T19:41:47Z

-                                        CompileLinkOptionsOrErr->first,
-                                        CompileLinkOptionsOrErr->second);
+      StringRef DeviceCompilerArgs =
+          LinkerArgs.getLastArgValue(OPT_compiler_arg_EQ);


Is this part combining the flags from the image with the command line flags from the tool? If so please add a comment

Yes, this is the part for combing the flags from the image with argument passed via option -device-compiler= and -device-linker= to clang-linker-warpper? I have added a comment in the code. Thank you! ;)

sarnex · 2026-02-19T19:42:27Z

+  // Filter the SYCL device compiler and linker options by target triple and
+  // offload kind.
+  const StringRef TripleStr = DAL.getLastArgValue(OPT_triple_EQ);
+  auto processDeviceArgs = [&](unsigned DeviceOptID, unsigned OptID) {


nit: usually we start lambda names with a capital letter

Thank you for pointing this out! I have made the change.

srividya-sundaram · 2026-02-19T22:04:19Z

+  // offload kind.
+  const StringRef TripleStr = DAL.getLastArgValue(OPT_triple_EQ);
+  auto processDeviceArgs = [&](unsigned DeviceOptID, unsigned OptID) {
+    for (StringRef Arg : Args.getAllArgValues(DeviceOptID)) {


nit: Rename variable Arg to DeviceArgValue or something similar as this is not an Arg*, but the values attached to the --device-compiler or --device-linker

Thank you for the suggestion! I have rename the variable to DeviceArgValue.

srividya-sundaram · 2026-02-19T22:17:39Z

+    for (StringRef Arg : Args.getAllArgValues(DeviceOptID)) {
+      size_t ColonPos = Arg.find(':');
+      if (ColonPos != StringRef::npos) {
+        StringRef Kind = Arg.substr(0, ColonPos);


nit: For better readability consider using drop_front and take_front functions instead of substr functions as appropriate.

I also agree that using drop_front and take_front would be better. I have made the change.

YixingZhang007 · 2026-02-25T16:41:29Z

I don’t think the current CI failure is related to the changes in this patch. The failures seems to be caused by the CI machine itself, and I am seeing similar issues with other PRs as well. Thank you!

mdtoguchi · 2026-02-25T16:56:15Z

+  auto ProcessDeviceArgs = [&](unsigned DeviceArgsOptionID,
+                               unsigned ForwardedOptionID) {


Suggested change

auto ProcessDeviceArgs = [&](unsigned DeviceArgsOptionID,

unsigned ForwardedOptionID) {

auto ProcessDeviceArgs = [&](llvm::opt::OptSpecifier DeviceArgsOptionID,

llvm::opt::OptSpecifier ForwardedOptionID) {

nit - while technically it is an unsigned, using OptSpecifier provides some clarity.

Thank you for the suggestion! I have made the changes :)

sarnex

looks good just some minor questions

sarnex · 2026-02-25T20:08:59Z

+                               llvm::opt::OptSpecifier ForwardedOptionID) {
+    for (StringRef DeviceArgValue : Args.getAllArgValues(DeviceArgsOptionID)) {
+      size_t ColonPos = DeviceArgValue.find(':');
+      if (ColonPos != StringRef::npos) {


Should we instead change all the != npos checks to be if ( == npos) continue? Or are there valid cases where some of those might not be present?

Thanks for the suggestion! Yes, there could be cases where the argument contains only <kind>:<value>, only <triple>=<value>, or even just <value> (both <kind> and <triple> are optional). If we use continue, it would skip cases where either <kind> or <triple> is not available.

got it, thanks. my third question seems related to this so ill ask a follow-up there

sarnex · 2026-02-25T20:11:12Z

+        // If this isn't a recognized triple then it's an `arg=value` option.
+        if (TT.getArch() != Triple::ArchType::UnknownArch) {
+          if (TargetTriple != TripleStr)
+            continue;


In what cases will the driver generate arg=value instead of including the triple?

There is an example found at

llvm/clang/test/Driver/linker-wrapper.c

Lines 145 to 148 in edd04ef

// RUN: clang-linker-wrapper --dry-run --host-triple=x86_64-unknown-linux-gnu \

// RUN: --linker-path=/usr/bin/ld --device-linker=foo=bar --device-linker=a \

// RUN: --device-linker=nvptx64-nvidia-cuda=b --device-compiler=foo\

// RUN: %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=LINKER-ARGS

In this example, foo=bar is passed as an argument to -device-linker, and foo is not an triple. I am not sure when this case would happen, and this is the only test found that tests this behavior. This line is added to allow this test case. Also, before this patch, we were also supporting the case of arg=value; removing it may make our code less aligned with the community code. Please feel free to let me know if there is any suggestion on this :) Thank you!

@mdtoguchi Do you think we need to support the above argument style? Or should we just change the test

The argument style can present itself externally - but albeit only with using the internal undocumented -Zlinker-input option. Weird case, but as community supports it we should as well.

sarnex

thanks!

YixingZhang007 · 2026-03-02T18:20:04Z

@intel/llvm-gatekeepers I don't think the CI failure is related to the change in this patch. Could you please help merge the PR? Thank you!

sarnex · 2026-03-02T18:22:18Z

Still needs a review from @intel/dpcpp-clang-driver-reviewers

sarnex · 2026-03-02T18:38:35Z

@YuriPlyakhin I think you need to remove your 'Requested changes' before we can merge (unless I should force merge)

YixingZhang007 · 2026-03-03T13:26:17Z

@YuriPlyakhin I think you need to remove your 'Requested changes' before we can merge (unless I should force merge)

@intel/llvm-gatekeepers I have removed the Requested changes as it has been addressed, and I think we should be able to merge this PR now. Thank you!

YixingZhang007 had a problem deploying to WindowsCILock January 14, 2026 17:31 — with GitHub Actions Failure

YixingZhang007 had a problem deploying to WindowsCILock January 14, 2026 18:04 — with GitHub Actions Failure

YixingZhang007 had a problem deploying to WindowsCILock January 16, 2026 21:04 — with GitHub Actions Failure

YixingZhang007 had a problem deploying to WindowsCILock January 16, 2026 21:33 — with GitHub Actions Failure

YixingZhang007 had a problem deploying to WindowsCILock January 16, 2026 21:59 — with GitHub Actions Error

YixingZhang007 had a problem deploying to WindowsCILock January 16, 2026 22:27 — with GitHub Actions Failure

YixingZhang007 had a problem deploying to WindowsCILock January 16, 2026 23:02 — with GitHub Actions Failure

YixingZhang007 had a problem deploying to WindowsCILock January 19, 2026 23:53 — with GitHub Actions Failure

YixingZhang007 had a problem deploying to WindowsCILock January 20, 2026 00:24 — with GitHub Actions Failure

YixingZhang007 had a problem deploying to WindowsCILock January 20, 2026 00:32 — with GitHub Actions Failure

YixingZhang007 had a problem deploying to WindowsCILock January 20, 2026 01:03 — with GitHub Actions Failure

YixingZhang007 had a problem deploying to WindowsCILock January 20, 2026 15:30 — with GitHub Actions Failure

YixingZhang007 had a problem deploying to WindowsCILock January 20, 2026 15:59 — with GitHub Actions Failure

YixingZhang007 had a problem deploying to WindowsCILock February 9, 2026 04:31 — with GitHub Actions Failure

YixingZhang007 had a problem deploying to WindowsCILock February 9, 2026 05:03 — with GitHub Actions Failure

YixingZhang007 had a problem deploying to WindowsCILock February 9, 2026 13:33 — with GitHub Actions Failure

YixingZhang007 temporarily deployed to WindowsCILock February 10, 2026 22:23 — with GitHub Actions Inactive

YixingZhang007 marked this pull request as ready for review February 10, 2026 22:56

YixingZhang007 requested review from a team as code owners February 10, 2026 22:56

YixingZhang007 requested review from YuriPlyakhin and srividya-sundaram February 10, 2026 22:57

YixingZhang007 temporarily deployed to WindowsCILock February 10, 2026 22:57 — with GitHub Actions Inactive

YixingZhang007 requested a review from maksimsab February 10, 2026 22:58

YuriPlyakhin previously requested changes Feb 11, 2026

View reviewed changes

Comment thread clang/test/Driver/linker-wrapper.c Outdated

Comment thread clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td

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

YixingZhang007 mentioned this pull request Feb 17, 2026

[NewOffloadModel] Add support for passing backend options for multiple device architectures #21140

Open

sarnex reviewed Feb 19, 2026

View reviewed changes

srividya-sundaram reviewed Feb 19, 2026

View reviewed changes

mdtoguchi reviewed Feb 25, 2026

View reviewed changes

sarnex reviewed Feb 25, 2026

View reviewed changes

sarnex approved these changes Mar 2, 2026

View reviewed changes

YixingZhang007 and others added 2 commits March 2, 2026 17:33

enable new offloading model by default

b27299b

refactor OPT_compiler_arg_EQ and OPT_linker_arg_EQ parsing

12caa6c

srividya-sundaram approved these changes Mar 2, 2026

View reviewed changes

YixingZhang007 mentioned this pull request Mar 3, 2026

[PLEASE DO NOT REVIEW] Revert change in PR #21055 #21426

Closed

		auto ProcessDeviceArgs = [&](unsigned DeviceArgsOptionID,
		unsigned ForwardedOptionID) {

	// RUN: clang-linker-wrapper --dry-run --host-triple=x86_64-unknown-linux-gnu \
	// RUN: --linker-path=/usr/bin/ld --device-linker=foo=bar --device-linker=a \
	// RUN: --device-linker=nvptx64-nvidia-cuda=b --device-compiler=foo\
	// RUN: %t.o -o a.out 2>&1 \| FileCheck %s --check-prefix=LINKER-ARGS

Conversation

YixingZhang007 commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

YixingZhang007 commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

YuriPlyakhin commented Feb 11, 2026

Uh oh!

YixingZhang007 commented Feb 18, 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!

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!

YixingZhang007 commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sarnex 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YixingZhang007 Feb 26, 2026 • 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!

sarnex left a comment

Choose a reason for hiding this comment

Uh oh!

YixingZhang007 commented Mar 2, 2026

Uh oh!

sarnex commented Mar 2, 2026

Uh oh!

sarnex commented Mar 2, 2026

Uh oh!

YixingZhang007 commented Mar 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

YixingZhang007 commented Jan 14, 2026 •

edited

Loading

YixingZhang007 commented Feb 10, 2026 •

edited

Loading

YixingZhang007 commented Feb 25, 2026 •

edited

Loading

YixingZhang007 Feb 26, 2026 •

edited

Loading