[SYCL] Support --offload-compress in clang-linker-wrapper#22605
Open
bviyer wants to merge 23 commits into
Open
[SYCL] Support --offload-compress in clang-linker-wrapper#22605bviyer wants to merge 23 commits into
bviyer wants to merge 23 commits into
Conversation
sarnex
reviewed
Jul 14, 2026
bviyer
force-pushed
the
bviyer-sycl-compress-support
branch
from
July 15, 2026 23:30
2361b30 to
ec31530
Compare
Contributor
|
@bviyer, please, remove |
Contributor
Author
Sorry. Fixed. |
bader
removed their request for review
July 15, 2026 23:52
sarnex
reviewed
Jul 16, 2026
sarnex
left a comment
sarnex
left a comment
Contributor
There was a problem hiding this comment.
nice cleanup! some comments below
Contributor
There was a problem hiding this comment.
i think we lost the LLVM_ENABLE_EXCEPTIONS handling which we probably need
sarnex
reviewed
Jul 16, 2026
sarnex
left a comment
sarnex
left a comment
Contributor
There was a problem hiding this comment.
did you forget to push your changes to the PR? i'm not seeing any new commits with fixes
sarnex
approved these changes
Jul 16, 2026
sarnex
left a comment
sarnex
left a comment
Contributor
There was a problem hiding this comment.
lgtm thanks! just some final nits
Forward --offload-compress (and --offload-compression-level=) from the driver to clang-linker-wrapper under the new offloading driver. In wrapSYCLBinariesFromFile, zstd-compress each SYCL device image whose payload is at or above --offload-compression-threshold= (default 512 bytes) and tag it with the new BIF_CompressedNone image format so the SYCL runtime decompresses lazily via CompressedRTDeviceBinaryImage.
ClangOffLoadWrappper and ClangLinkerWrapper.
bviyer
force-pushed
the
bviyer-sycl-compress-support
branch
from
July 16, 2026 22:19
e45280d to
e79b396
Compare
mdtoguchi
approved these changes
Jul 16, 2026
YuriPlyakhin
left a comment
YuriPlyakhin
left a comment
Contributor
There was a problem hiding this comment.
Please update PR description: BIF_CompressedNone -> BIF_Compressed
YuriPlyakhin
left a comment
YuriPlyakhin
left a comment
Contributor
There was a problem hiding this comment.
Could you please add a functional test for the new compression path to test changes in clang-linker-wrapper.cpp?
Test would run clang-linker-wrapper --compress and would verify the emitted image is tagged with BIF_Compressed.
Also it would check invalid value for --offload-compression-level= error message.
FYI: The old wrapper is tested by clang-offload-wrapper-zstd.c
sarnex
reviewed
Jul 17, 2026
Contributor
|
@intel/llvm-gatekeepers please consider merging |
YuriPlyakhin
approved these changes
Jul 18, 2026
YuriPlyakhin
left a comment
YuriPlyakhin
left a comment
Contributor
There was a problem hiding this comment.
LGTM. Few nits - all related to too verbose comments. Feel free to ignore. Thanks! :)
Co-authored-by: Yury Plyakhin <yury.plyakhin@intel.com>
Co-authored-by: Yury Plyakhin <yury.plyakhin@intel.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds SYCL device-image compression support to the new offloading driver path by forwarding --offload-compress (and --offload-compression-level=) to clang-linker-wrapper, then zstd-compressing SYCL images in wrapSYCLBinariesFromFile and tagging them as BIF_Compressed so the SYCL runtime can lazily decompress them.
Changes:
- Introduces
offloading::compressSYCLDeviceImage()as a shared helper for zstd compression with threshold/verbosity support. - Extends SYCL wrapper image-format handling with a new
SYCLBinaryImageFormat::BIF_Compressed(mapped to int8 value4). - Adds
clang-linker-wrapperin-place compression of SYCL device images when--compressis present, plus new driver and wrapper tests.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| llvm/lib/Frontend/Offloading/Utility.cpp | Implements compressSYCLDeviceImage() using LLVM zstd support and verbose size reporting. |
| llvm/include/llvm/Frontend/Offloading/Utility.h | Declares compressSYCLDeviceImage() API and documents parameters/defaults. |
| llvm/lib/Frontend/Offloading/SYCLOffloadWrapper.cpp | Maps new BIF_Compressed to serialized int8 format value. |
| llvm/include/llvm/Frontend/Offloading/SYCLOffloadWrapper.h | Adds BIF_Compressed to the SYCL image-format enum. |
| clang/tools/clang-offload-wrapper/CMakeLists.txt | Links clang-offload-wrapper against FrontendOffloading to use the shared helper. |
| clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp | Switches compression logic to call the shared helper; renames kind enum to OldOffloadKind. |
| clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp | Adds --compress path that compresses SYCL images and tags them BIF_Compressed. |
| clang/test/Driver/sycl-offload-new-driver-compression.cpp | Verifies driver forwarding of compression flags to clang-linker-wrapper. |
| clang/test/Driver/clang-linker-wrapper-zstd.cpp | End-to-end wrapper test: compression happens, format tagged, level parsing and diagnostics. |
Comment on lines
+1119
to
+1123
| if (auto *A = Args.getLastArg(OPT_compression_level_eq)) | ||
| if (StringRef(A->getValue()).getAsInteger(10, Level)) | ||
| return createStringError( | ||
| "invalid value for --offload-compression-level=: '%s'", | ||
| A->getValue()); |
Comment on lines
+1125
to
+1132
| for (auto &Image : Images) { | ||
| SmallVector<uint8_t, 0> CompressedBytes; | ||
| Expected<bool> DidCompressOrErr = offloading::compressSYCLDeviceImage( | ||
| ArrayRef<uint8_t>( | ||
| reinterpret_cast<const uint8_t *>(Image.Image->getBufferStart()), | ||
| Image.Image->getBufferSize()), | ||
| CompressedBytes, Level, /*Threshold=*/512, Verbose); | ||
| if (!DidCompressOrErr) |
Comment on lines
83
to
92
| // Offload models supported by this tool. The support basically means mapping | ||
| // a string representation given at the command line to a value from this | ||
| // enum. | ||
| enum OffloadKind { | ||
| // enum. Distinct from llvm::object::OffloadKind (in Object/OffloadBinary.h), | ||
| // which is the 16-bit bitmask stored in the OffloadBinary header | ||
| // (OFK_None/OFK_OpenMP/... = powers of two). This enum is a sequential | ||
| // command-line-parsed tag with an extra Host slot and First/Last sentinels, | ||
| // serialized as a uint8_t into the SYCL runtime's sycl_device_binary_struct | ||
| // Kind field. | ||
| enum OldOffloadKind { | ||
| Unknown = 0, |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Yury Plyakhin <yury.plyakhin@intel.com>
Co-authored-by: Srividya Sundaram <srividya.sundaram@intel.com>
Co-authored-by: Srividya Sundaram <srividya.sundaram@intel.com>
Co-authored-by: Srividya Sundaram <srividya.sundaram@intel.com>
…to bviyer-sycl-compress-support
| @@ -0,0 +1,77 @@ | |||
| // REQUIRES: zstd && system-linux && x86-registered-target | |||
|
|
|||
| // clang-linker-wrapper compression test: checks that the wrapper compresses | |||
Contributor
There was a problem hiding this comment.
nit: This file tests clang-linker-wrapper functionality, not driver-specific behavior. Might be better if it is relocated out of the driver test directory and into the appropriate linker-wrapper test directory.
srividya-sundaram
approved these changes
Jul 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Forward --offload-compress (and --offload-compression-level=) from the driver to clang-linker-wrapper under the new offloading driver. In wrapSYCLBinariesFromFile, zstd-compress each SYCL device image whose payload is at or above --offload-compression-threshold= (default 512 bytes) and tag it with the new BIF_Compressed image format so the SYCL runtime decompresses lazily via CompressedRTDeviceBinaryImage.
Significant help from Claude Opus 4.7.