-
Notifications
You must be signed in to change notification settings - Fork 752
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
Changes from 56 commits
ef323f7
bdab2f0
45f1e99
34978f8
195e961
cd64225
d89f41b
fb643e3
151e70a
2983fab
054984c
4493984
dbb96a7
6c26a42
7d7edc6
f0aca25
dbda582
758723c
7282eec
eda727e
94a98c9
6cc23ec
4297d5f
71abfce
fae7906
9272d65
b4a2c0c
fa9459b
63389bb
6487867
5099ee9
687db23
ff53221
7fb726e
84f0864
7fdbd5e
312bd38
75f28ac
288ed5b
e3576a6
b6f1f7a
7262fd1
6ff1a37
022a7ef
2eb0c25
44b41dd
1d81813
9936bab
969b520
32e4868
fd0f1e3
07c119e
eb7588c
58f9939
575efc6
970ad35
c1a2c13
966e3dd
946a738
ce1d0f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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: |
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"{{.*}} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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. | ||
|
@@ -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"); | ||
|
||
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if the compression fails? (for example, out of disk space) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If compression fails, I think it's better to throw and exit, then using the uncompressed image. This behaviour aligns with compression in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What kind of message is provided to the user if compression fails? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here's LLVM's ZSTD compression function:
LLVM throws the appropriate error message, depending on the type of 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When the crash occurs when compression failed, the error message that the SYCL end-user will receive may not be useful. |
||
|
||
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) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we shouldn't turn on these by default? We should just turn on them in
if args.ci_defaults:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea behind turning them on by default is to build the compiler with device image compression support if the user has zstd-dev package installed. If zstd is not found, there shouldn't be any build error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are configurate failures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CMake Error at lib/Support/CMakeLists.txt:327 (get_property):
get_property could not find TARGET zstd::libzstd_static. Perhaps it has
not yet been created.
CMake Error at lib/Support/CMakeLists.txt:330 (get_property):
get_property could not find TARGET zstd::libzstd_static. Perhaps it has
not yet been created.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah local build is cooked for me too, we need to revert the on by zstd default part at least
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR to temporarily turn off zstd by default: #15833