Skip to content
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.

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

[SYCL][Clang] Add support for device image compression #15124

Open
wants to merge 56 commits into
base: sycl
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
ef323f7
Add sycl-compress
uditagarwal97 Aug 10, 2024
bdab2f0
Fix decompression in RT
uditagarwal97 Aug 17, 2024
45f1e99
Cleanup
uditagarwal97 Aug 17, 2024
34978f8
Fix ZSTD Cmake dependencies
uditagarwal97 Aug 18, 2024
195e961
Merge branch 'sycl' into compress_img
uditagarwal97 Aug 18, 2024
cd64225
Remove unwanted formatting changes
uditagarwal97 Aug 18, 2024
d89f41b
More cleanup
uditagarwal97 Aug 18, 2024
fb643e3
Add option in clang driver to trigger compression.
uditagarwal97 Aug 18, 2024
151e70a
Cleanup + build fix
uditagarwal97 Aug 19, 2024
2983fab
Fix ZSTD build on windows, RHEL
uditagarwal97 Aug 19, 2024
054984c
Merge remote-tracking branch 'upstream/sycl' into compress_img
uditagarwal97 Aug 19, 2024
4493984
Fix clang warnings and formatting
uditagarwal97 Aug 19, 2024
dbb96a7
Try fixing Windows build
uditagarwal97 Aug 20, 2024
6c26a42
Merge remote-tracking branch 'upstream/sycl' into compress_img
uditagarwal97 Aug 26, 2024
7d7edc6
Fix linkage error while windows build
uditagarwal97 Aug 26, 2024
f0aca25
Fix include_directory for sycl-compress
uditagarwal97 Aug 26, 2024
dbda582
Merge remote-tracking branch 'upstream/sycl' into compress_img
uditagarwal97 Aug 26, 2024
758723c
Fix formatting. Remove redundant incude_directory in CMakeFiles
uditagarwal97 Aug 26, 2024
7282eec
Another test
uditagarwal97 Aug 27, 2024
eda727e
Another attempt at fixing win build
uditagarwal97 Aug 27, 2024
94a98c9
Explicitly include sycl-compress headers
uditagarwal97 Aug 27, 2024
6cc23ec
Reuse context in sycl-compress; Pass unique_ptr to caller instead of …
uditagarwal97 Aug 27, 2024
4297d5f
Add unit tests and performance tests for sycl-compress
uditagarwal97 Aug 28, 2024
71abfce
Reuse already existing clang options for compression. Use LLVMSupport…
uditagarwal97 Aug 31, 2024
fae7906
Use LLVMSupport in SYCL RT to decompress; Remove sycl-compress librar…
uditagarwal97 Aug 31, 2024
9272d65
Fix build error when zstd is not present.
uditagarwal97 Sep 1, 2024
b4a2c0c
link zstd with RT instead of LLVMSupport
uditagarwal97 Sep 6, 2024
fa9459b
Merge remote-tracking branch 'upstream/sycl' into compress_img
uditagarwal97 Sep 6, 2024
63389bb
Fix formatting; Add CMAKE_FIND_DEBUG_MODE to debug why zstd is not fo…
uditagarwal97 Sep 6, 2024
6487867
Add LIT test for driver changes.
uditagarwal97 Sep 7, 2024
5099ee9
Merge remote-tracking branch 'upstream/sycl' into compress_img
uditagarwal97 Sep 7, 2024
687db23
Revert changes in llvm::compression namespace.
uditagarwal97 Sep 7, 2024
ff53221
Add unit test for (de)compression
uditagarwal97 Sep 7, 2024
7fb726e
Add E2E test for image compression
uditagarwal97 Sep 8, 2024
84f0864
Update doc; Remove warning.
uditagarwal97 Sep 8, 2024
7fdbd5e
Add line at EOF
uditagarwal97 Sep 8, 2024
312bd38
Remove std::move to allow copy ellision; Moved header to source/detail
uditagarwal97 Sep 9, 2024
75f28ac
Simplify passing argument from clang driver to clang-offload-wrapper
uditagarwal97 Sep 9, 2024
288ed5b
Fix formatting
uditagarwal97 Sep 10, 2024
e3576a6
Merge remote-tracking branch 'upstream/sycl' into compress_img
uditagarwal97 Sep 10, 2024
b6f1f7a
Merge remote-tracking branch 'upstream/sycl' into compress_img
uditagarwal97 Sep 11, 2024
7262fd1
Address reviews
uditagarwal97 Sep 11, 2024
6ff1a37
Add E2E tests for image compression.
uditagarwal97 Sep 12, 2024
022a7ef
Remove pending TODOs; Apply clang-format
uditagarwal97 Sep 12, 2024
2eb0c25
Reuse LLVM_ENABLE_ZSTD
uditagarwal97 Sep 13, 2024
44b41dd
Find zstd package when we build E2E tests seperatly from the compiler
uditagarwal97 Sep 13, 2024
1d81813
Throw error in clang-offload-wrapper when zstd is not present but sti…
uditagarwal97 Sep 13, 2024
9936bab
Add debug mode to Finezstd.cmake
uditagarwal97 Sep 14, 2024
969b520
Fix compression_seperate_compile test failure
uditagarwal97 Sep 15, 2024
32e4868
Fix unreferenced var error on MSVC; Remove debug prints.
uditagarwal97 Sep 15, 2024
fd0f1e3
Simply E2E test and fix failure on CUDA
uditagarwal97 Sep 17, 2024
07c119e
Merge remote-tracking branch 'origin/sycl' into HEAD
uditagarwal97 Sep 17, 2024
eb7588c
Address reviews
uditagarwal97 Sep 17, 2024
58f9939
Add clang driver test. Address reviews.
uditagarwal97 Sep 18, 2024
575efc6
Fix detection of zstd LIT feature on Windows
uditagarwal97 Sep 18, 2024
970ad35
Simplify zstd detection in LIT; Remove ZSTD requirement in clang driv…
uditagarwal97 Sep 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions buildbot/configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,8 @@ def do_configure(args):
"-DLLVM_ENABLE_PROJECTS={}".format(llvm_enable_projects),
"-DSYCL_BUILD_PI_HIP_PLATFORM={}".format(sycl_build_pi_hip_platform),
"-DLLVM_BUILD_TOOLS=ON",
"-DLLVM_ENABLE_ZSTD=ON",
"-DLLVM_USE_STATIC_ZSTD=ON",
"-DSYCL_ENABLE_WERROR={}".format(sycl_werror),
"-DCMAKE_INSTALL_PREFIX={}".format(install_dir),
"-DSYCL_INCLUDE_TESTS=ON", # Explicitly include all kinds of SYCL tests.
Expand Down
13 changes: 13 additions & 0 deletions clang/lib/Driver/ToolChains/Clang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10131,6 +10131,19 @@ void OffloadWrapper::ConstructJob(Compilation &C, const JobAction &JA,
SmallString<128> TargetTripleOpt = TT.getArchName();
bool WrapFPGADevice = false;
bool FPGAEarly = false;

// Validate and propogate CLI options related to device image compression.
// -offload-compress
if (C.getInputArgs().getLastArg(options::OPT_offload_compress)) {
WrapperArgs.push_back(
C.getArgs().MakeArgString(Twine("-offload-compress")));
// -offload-compression-level=<>
if (Arg *A = C.getInputArgs().getLastArg(
options::OPT_offload_compression_level_EQ))
WrapperArgs.push_back(C.getArgs().MakeArgString(
Twine("-offload-compression-level=") + A->getValue()));
}
uditagarwal97 marked this conversation as resolved.
Show resolved Hide resolved

if (Arg *A = C.getInputArgs().getLastArg(options::OPT_fsycl_link_EQ)) {
WrapFPGADevice = true;
FPGAEarly = (A->getValue() == StringRef("early"));
Expand Down
40 changes: 40 additions & 0 deletions clang/test/Driver/clang-offload-wrapper-zstd.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// REQUIRES: zstd && (system-windows || system-linux)

// clang-offload-wrapper compression test: checks that the wrapper can compress the device images.
// Checks the '--offload-compress', '--offload-compression-level', and '--offload-compression-threshold'
// CLI options.

// --- Prepare test data by creating the debice binary image.
// RUN: echo -e -n 'device binary image1\n' > %t.bin
// RUN: echo -e -n '[Category1]\nint_prop1=1|10\n[Category2]\nint_prop2=1|20\n' > %t.props
// RUN: echo -e -n 'kernel1\nkernel2\n' > %t.sym
// RUN: echo -e -n 'Manifest file - arbitrary data generated by the toolchain\n' > %t.mnf
// RUN: echo '[Code|Properties|Symbols|Manifest]' > %t.img1
// RUN: echo %t.bin"|"%t.props"|"%t.sym"|"%t.mnf >> %t.img1

///////////////////////////////////////////////////////
// Compress the test image using clang-offload-wrapper.
///////////////////////////////////////////////////////

// RUN: clang-offload-wrapper -kind=sycl -target=TARGET -batch %t.img1 -o %t.wrapped.bc -v \
// RUN: --offload-compress --offload-compression-level=9 --offload-compression-threshold=0 \
// RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-COMPRESS

// CHECK-COMPRESS: [Compression] Original image size:
// CHECK-COMPRESS: [Compression] Compressed image size:
// CHECK-COMPRESS: [Compression] Compression level used: 9

///////////////////////////////////////////////////////////
// Check that there is no compression when the threshold is set to a value higher than the image size
// or '--offload-compress' is not set.
///////////////////////////////////////////////////////////

// RUN: clang-offload-wrapper -kind=sycl -target=TARGET -batch %t.img1 -o %t.wrapped.bc -v \
// RUN: --offload-compress --offload-compression-level=3 --offload-compression-threshold=1000 \
// RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-NO-COMPRESS

// RUN: clang-offload-wrapper -kind=sycl -target=TARGET -batch %t.img1 -o %t.wrapped.bc -v \
// RUN: --offload-compression-level=3 --offload-compression-threshold=0 \
// RUN: 2>&1 | FileCheck %s --check-prefix=CHECK-NO-COMPRESS

// CHECK-NO-COMPRESS-NOT: [Compression] Original image size:
14 changes: 14 additions & 0 deletions clang/test/Driver/sycl-offload-wrapper-compression.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
///
/// Check if '--offload-compress' and '--offload-compression-level' CLI
/// options are passed to the clang-offload-wrapper.
///

// RUN: %clangxx -### -fsycl --offload-compress --offload-compression-level=3 %s 2>&1 | FileCheck %s --check-prefix=CHECK-COMPRESS
// CHECK-COMPRESS: {{.*}}clang-offload-wrapper{{.*}}"-offload-compress"{{.*}}"-offload-compression-level=3"{{.*}}

// Make sure that the compression options are not passed when --offload-compress is not set.
// RUN: %clangxx -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=CHECK-NO-COMPRESS
// RUN: %clangxx -### -fsycl --offload-compression-level=3 %s 2>&1 | FileCheck %s --check-prefix=CHECK-NO-COMPRESS

// CHECK-NO-COMPRESS-NOT: {{.*}}clang-offload-wrapper{{.*}}"-offload-compress"{{.*}}
// CHECK-NO-COMPRESS-NOT: {{.*}}clang-offload-wrapper{{.*}}"-offload-compression-level=3"{{.*}}
1 change: 1 addition & 0 deletions clang/tools/clang-offload-wrapper/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ add_clang_tool(clang-offload-wrapper

set(CLANG_OFFLOAD_WRAPPER_LIB_DEPS
clangBasic
LLVMSupport
)

add_dependencies(clang clang-offload-wrapper)
Expand Down
83 changes: 77 additions & 6 deletions clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@
#include <string>
#include <tuple>

// For device image compression.
#include <llvm/Support/Compression.h>

#define OPENMP_OFFLOAD_IMAGE_VERSION "1.0"

using namespace llvm;
Expand Down Expand Up @@ -139,15 +142,35 @@ static cl::list<std::string> Inputs(cl::Positional, cl::OneOrMore,
cl::desc("<input files>"),
cl::cat(ClangOffloadWrapperCategory));

// CLI options for device image compression.
static cl::opt<bool> OffloadCompressDevImgs(
"offload-compress", cl::init(false), cl::Optional,
cl::desc("Enable device image compression using ZSTD."),
cl::cat(ClangOffloadWrapperCategory));

static cl::opt<int>
OffloadCompressLevel("offload-compression-level", cl::init(10),
cl::Optional,
cl::desc("ZSTD Compression level. Default: 10"),
cl::cat(ClangOffloadWrapperCategory));

static cl::opt<int>
OffloadCompressThreshold("offload-compression-threshold", cl::init(512),
cl::Optional,
cl::desc("Threshold (in bytes) over which to "
"compress images. Default: 512"),
cl::cat(ClangOffloadWrapperCategory));

// Binary image formats supported by this tool. The support basically means
// mapping string representation given at the command line to a value from this
// enum. No format checking is performed.
enum BinaryImageFormat {
none, // image kind is not determined
native, // image kind is native
// portable image kinds go next
spirv, // SPIR-V
llvmbc // LLVM bitcode
spirv, // SPIR-V
llvmbc, // LLVM bitcode
compressed_none // compressed image with unknown format
};

/// Sets offload kind.
Expand Down Expand Up @@ -265,6 +288,8 @@ static StringRef formatToString(BinaryImageFormat Fmt) {
return "llvmbc";
case BinaryImageFormat::native:
return "native";
case BinaryImageFormat::compressed_none:
return "compressed_none";
}
llvm_unreachable("bad format");

Expand Down Expand Up @@ -1083,10 +1108,56 @@ class BinaryWrapper {
return FBinOrErr.takeError();
Fbin = *FBinOrErr;
} else {
Fbin = addDeviceImageToModule(
ArrayRef<char>(Bin->getBufferStart(), Bin->getBufferSize()),
Twine(OffloadKindTag) + Twine(ImgId) + Twine(".data"), Kind,
Img.Tgt);

// If '--offload-compress' option is specified and zstd is not
// available, throw an error.
if (OffloadCompressDevImgs && !llvm::compression::zstd::isAvailable()) {
createStringError(inconvertibleErrorCode(),
"'--offload-compress' option is specified but zstd "
"is not available. The device image will not be "
"compressed.");
}

// Don't compress if the user explicitly specifies the binary image
// format or if the image is smaller than OffloadCompressThreshold
// bytes.
if (Kind != OffloadKind::SYCL || !OffloadCompressDevImgs ||
Img.Fmt != BinaryImageFormat::none ||
!llvm::compression::zstd::isAvailable() ||
static_cast<int>(Bin->getBufferSize()) < OffloadCompressThreshold) {
Fbin = addDeviceImageToModule(
ArrayRef<char>(Bin->getBufferStart(), Bin->getBufferSize()),
Twine(OffloadKindTag) + Twine(ImgId) + Twine(".data"), Kind,
Img.Tgt);
} else {

// Compress the image using zstd.
SmallVector<uint8_t, 512> CompressedBuffer;
llvm::compression::zstd::compress(
ArrayRef<unsigned char>(
(const unsigned char *)(Bin->getBufferStart()),
Bin->getBufferSize()),
CompressedBuffer, OffloadCompressLevel);
Comment on lines +1136 to +1140
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if the compression fails? (for example, out of disk space)
Can we catch this kind of exception and gracefully use an uncompressed image? (warning message would be helpful)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

llvm::compression::zstd::compress itself do error checking and throw if ran out of space or unsuccessful compression: https://llvm.org/docs/doxygen/Compression_8cpp_source.html.

If compression fails, I think it's better to throw and exit, then using the uncompressed image. This behaviour aligns with compression in clang-offload-bundler (what we have upstream).

Copy link
Contributor

Choose a reason for hiding this comment

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

What kind of message is provided to the user if compression fails?
We usually surround a call to a 3rd party library call with try-catch block so we can gracefully exit with some explanation about the error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's LLVM's ZSTD compression function:

void zstd::compress(ArrayRef<uint8_t> Input,
                    SmallVectorImpl<uint8_t> &CompressedBuffer, int Level,
                    bool EnableLdm) {
  ZSTD_CCtx *Cctx = ZSTD_createCCtx();
  if (!Cctx)
    report_bad_alloc_error("Failed to create ZSTD_CCtx");

  if (ZSTD_isError(ZSTD_CCtx_setParameter(
          Cctx, ZSTD_c_enableLongDistanceMatching, EnableLdm ? 1 : 0))) {
    ZSTD_freeCCtx(Cctx);
    report_bad_alloc_error("Failed to set ZSTD_c_enableLongDistanceMatching");
  }

  if (ZSTD_isError(
          ZSTD_CCtx_setParameter(Cctx, ZSTD_c_compressionLevel, Level))) {
    ZSTD_freeCCtx(Cctx);
    report_bad_alloc_error("Failed to set ZSTD_c_compressionLevel");
  }

  unsigned long CompressedBufferSize = ZSTD_compressBound(Input.size());
  CompressedBuffer.resize_for_overwrite(CompressedBufferSize);

  size_t const CompressedSize =
      ZSTD_compress2(Cctx, CompressedBuffer.data(), CompressedBufferSize,
                     Input.data(), Input.size());

  ZSTD_freeCCtx(Cctx);

  if (ZSTD_isError(CompressedSize))
    report_bad_alloc_error("Compression failed");

  __msan_unpoison(CompressedBuffer.data(), CompressedSize);
  if (CompressedSize < CompressedBuffer.size())
    CompressedBuffer.truncate(CompressedSize);
}

LLVM throws the appropriate error message, depending on the type of error.

We usually surround a call to a 3rd party library call with try-catch block so we can gracefully exit with some explanation about the error.

While I totally agree with it but since we are using upstream code for image compression, I don't think we should deviate from the upstream in terms of error handling.


if (Verbose)
errs() << "[Compression] Original image size: "
<< Bin->getBufferSize() << "\n"
<< "[Compression] Compressed image size: "
<< CompressedBuffer.size() << "\n"
<< "[Compression] Compression level used: "
<< OffloadCompressLevel << "\n";

// Add the compressed image to the module.
Fbin = addDeviceImageToModule(
ArrayRef<char>((const char *)CompressedBuffer.data(),
CompressedBuffer.size()),
Twine(OffloadKindTag) + Twine(ImgId) + Twine(".data"), Kind,
Img.Tgt);

// Change image format to compressed_none.
Ffmt = ConstantInt::get(Type::getInt8Ty(C),
BinaryImageFormat::compressed_none);
}
}

if (Kind == OffloadKind::SYCL) {
Expand Down
13 changes: 13 additions & 0 deletions sycl/doc/UsersManual.md
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,19 @@ and not recommended to use in production environment.
which may or may not perform additional inlining.
Default value is 225.

**`--offload-compress`**

Enables device image compression for SYCL offloading. Device images
are compressed using `zstd` compression algorithm and only if their size
exceeds 512 bytes.
Default value is false.

**`--offload-compression-level=<int>`**

`zstd` compression level used to compress device images when `--offload-
compress` is enabled.
The default value is 10.

## Target toolchain options

**`-Xsycl-target-backend=<T> "options"`**
Expand Down
7 changes: 7 additions & 0 deletions sycl/source/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,13 @@ function(add_sycl_rt_library LIB_NAME LIB_OBJ_NAME)
target_link_libraries(${LIB_NAME} PRIVATE ${ARG_XPTI_LIB})
endif()

if (NOT LLVM_ENABLE_ZSTD)
target_compile_definitions(${LIB_OBJ_NAME} PRIVATE SYCL_RT_ZSTD_NOT_AVAIABLE)
else()
target_link_libraries(${LIB_NAME} PRIVATE ${zstd_STATIC_LIBRARY})
target_include_directories(${LIB_OBJ_NAME} PRIVATE ${zstd_INCLUDE_DIR})
endif()

target_include_directories(${LIB_OBJ_NAME} PRIVATE ${BOOST_UNORDERED_INCLUDE_DIRS})

# ur_win_proxy_loader
Expand Down
3 changes: 2 additions & 1 deletion sycl/source/detail/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ enum sycl_device_binary_type : uint8_t {
SYCL_DEVICE_BINARY_TYPE_NONE = 0, // undetermined
SYCL_DEVICE_BINARY_TYPE_NATIVE = 1, // specific to a device
SYCL_DEVICE_BINARY_TYPE_SPIRV = 2,
SYCL_DEVICE_BINARY_TYPE_LLVMIR_BITCODE = 3
SYCL_DEVICE_BINARY_TYPE_LLVMIR_BITCODE = 3,
SYCL_DEVICE_BINARY_TYPE_COMPRESSED_NONE = 4
};

// Device binary descriptor version supported by this library.
Expand Down
Loading
Loading