Skip to content

Conversation

@YuriPlyakhin
Copy link
Contributor

@YuriPlyakhin YuriPlyakhin commented Sep 11, 2025

This change main purpose is to align offload binary descriptor with the one we are upstreaming to llorg.

  • Added the -fpreview-breaking-changes flag to both the clang-offload-wrapper and clang-linker-wrapper tools, allowing users to enable preview features
  • Updated offload binary structure to remove the manifest pointers (ABI breaking change) and adjust the device image struct version when preview mode is enabled.
  • Modified batch mode and manifest handling so that, under preview-breaking-changes, the manifest column is omitted from batch files and related code paths. Documentation and code comments have been updated accordingly.
  • Added a new test file clang-offload-wrapper-exe-preview.cpp to verify the correct behavior of the wrapper tool when preview-breaking-changes are enabled. Existing tests have been annotated to clarify their usage in preview and non-preview modes.
  • Propagated the preview-breaking-changes flag through the clang driver and linker wrapper, ensuring that the new format and behaviors are activated when the flag is present.
  • Removed unused variables and cleaned up code paths related to image suffix handling in the linker wrapper.
    These changes collectively enable opt-in support for upcoming ABI breaking changes to the SYCL offload binary format, improve maintainability, and provide robust test coverage for both legacy and preview modes.


StringRef Prefix =
sys::path::stem(Binary.getMemoryBufferRef().getBufferIdentifier());
StringRef Suffix = getImageKindName(Binary.getImageKind());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not used. removed.

Copy link
Contributor

@srividya-sundaram srividya-sundaram left a comment

Choose a reason for hiding this comment

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

Driver changes LGTM.

Copy link
Contributor

@maarquitos14 maarquitos14 left a comment

Choose a reason for hiding this comment

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

LGTM, just a couple nits.

@github-actions
Copy link
Contributor

@intel/llvm-gatekeepers please consider merging

@YuriPlyakhin
Copy link
Contributor Author

@intel/llvm-gatekeepers please consider merging

@intel/llvm-gatekeepers , please, do not merge yet. Not approved yet by breaking change team.

@YuriPlyakhin
Copy link
Contributor Author

YuriPlyakhin commented Sep 24, 2025

@cperkinsintel , @aelovikov-intel , ping. Breaking change team has approved it, so I want to merge.

@YuriPlyakhin
Copy link
Contributor Author

@intel/llvm-gatekeepers , all green, please merged.

@aelovikov-intel aelovikov-intel merged commit e59fe8e into intel:sycl Sep 24, 2025
43 of 44 checks passed
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.

4 participants