Skip to content

[Driver] Fix preview breaking changes option passing to offload-wrapper #20708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Dec 2, 2025
Merged

[Driver] Fix preview breaking changes option passing to offload-wrapper #20708

merged 4 commits into from
Dec 2, 2025

Conversation

@mdtoguchi
Copy link
Contributor

@mdtoguchi mdtoguchi commented Nov 20, 2025 •
edited
Loading

When passing options to the clang-offload-wrapper, the -fpreview-breaking-changes option needs to be passed accordingly to the tool when -fpreview-breaking-changes is used on the command line. This was happening for typical SYCL enabled compilations, but when using -fsycl-link, the usage of clang-offload-wrapper modifies the offloading kind encountered, causing the option to not be passed.

Fixup the ability to pass -fpreview-breaking-changes for all needed uses as well as factoring in the offload-compress options which are in the same bucket of behaviors. This is done by creating a common function that adds the options via CLI enabling that is used for both -fsycl-link and non -fsycl-link usage paths.

@mdtoguchi
[Driver] Fix preview breaking changes option passing to offload-wrapper
4d1b019 
When passing options to the clang-offload-wrapper, the
-fpreview-breaking-changes option needs to be passed accordingly to the
tool when -fpreview-breaking-changes is used on the command line.  This
was happening for typical SYCL enabled compilations, but when using
-fsycl-link, the usage of clang-offload-wrapper modifies the offloading
kind encountered, causing the option to not be passed.

Fixup the ability to pass -fpreview-breaking-changes for all needed uses
as well as factoring in the offload-compress options which are in the
same bucket of behaviors.
@mdtoguchi mdtoguchi requested a review from a team as a code owner November 20, 2025 14:58
@mdtoguchi
Copy link
Contributor Author

mdtoguchi commented Nov 25, 2025

@intel/dpcpp-clang-driver-reviewers , please take a look.

@mdtoguchi mdtoguchi mentioned this pull request Nov 25, 2025
Merged
hchilama
hchilama approved these changes Nov 25, 2025
View reviewed changes
Copy link
Contributor

@hchilama hchilama left a comment

Choose a reason for hiding this comment

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

Do we have a test case checking for offload-compress and offload-compression-level= ? if so LGTM

@srividya-sundaram
Copy link
Contributor

srividya-sundaram commented Nov 25, 2025

but when using -fsycl-link, the usage of clang-offload-wrapper modifies the offloading kind encountered, causing the option to not be passed.

Is there a test case for this?

srividya-sundaram
srividya-sundaram reviewed Nov 25, 2025
View reviewed changes
srividya-sundaram
srividya-sundaram reviewed Nov 25, 2025
View reviewed changes
@srividya-sundaram
Copy link
Contributor

srividya-sundaram commented Nov 25, 2025

When passing options to the clang-offload-wrapper, the -fpreview-breaking-changes option needs to be passed accordingly to the tool when -fpreview-breaking-changes is used on the command line. This was happening for typical SYCL enabled compilations, but when using -fsycl-link, the usage of clang-offload-wrapper modifies the offloading kind encountered, causing the option to not be passed.

Fixup the ability to pass -fpreview-breaking-changes for all needed uses as well as factoring in the offload-compress options which are in the same bucket of behaviors.

Please update the description to also mention the changes related to : Validate and propogate CLI options related to device image compression

@srividya-sundaram
Copy link
Contributor

srividya-sundaram commented Nov 25, 2025

Precommit testing has this failure:

Failed Tests (1):
  SYCL :: DeviceLib/separate_compile_test.cpp

@mdtoguchi
Merge remote-tracking branch 'intel_llvm/sycl' into preview-break-wra…
d127c2d 
…pper
@mdtoguchi
Update logic for --offload-compress check
f58ec88 
Update the logic for the --offload-compress recognition on the command line
and update tests to check the proper behavior with -fsycl-link.
@mdtoguchi
Copy link
Contributor Author

mdtoguchi commented Dec 1, 2025

Do we have a test case checking for offload-compress and offload-compression-level= ? if so LGTM

Yes, this test does exist in test/sycl-offload-wrapper-compression.cpp. I've also updated this test to verify -fsycl-link behaviors.

@mdtoguchi
Copy link
Contributor Author

mdtoguchi commented Dec 1, 2025

but when using -fsycl-link, the usage of clang-offload-wrapper modifies the offloading kind encountered, causing the option to not be passed.

Is there a test case for this?

I have updated sycl-offload-wrapper-compression.cpp to test this.

@mdtoguchi
Copy link
Contributor Author

mdtoguchi commented Dec 1, 2025

@intel/llvm-gatekeepers, please consider for merge. Failures are not related to this PR.

@steffenlarsen steffenlarsen merged commit 8ae03ed into intel:sycl Dec 2, 2025
59 of 62 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@srividya-sundaram srividya-sundaram srividya-sundaram approved these changes

@hchilama hchilama hchilama approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mdtoguchi @srividya-sundaram @hchilama @steffenlarsen