[PATCH] D128667: [WIP] Add Zstd ELF support

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

The lld/ change should be separate. And it needs tests. We should have several tests like mixed zlib and zstd input. OutputSections should not have a new member. I can handle the lld/ELF part.

> Context not available.

See https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface "To make reviews easier, please always include as much context as possible with your diff!"



================
Comment at: llvm/include/llvm/BinaryFormat/ELF.h:1772
   ELFCOMPRESS_ZLIB = 1,            // ZLIB/DEFLATE algorithm.
+  ELFCOMPRESS_ZSTD = 2,            // ZLIB/DEFLATE algorithm.
   ELFCOMPRESS_LOOS = 0x60000000,   // Start of OS-specific.
----------------
Zstandard algorithm


================
Comment at: llvm/lib/MC/ELFObjectWriter.cpp:149
                              SmallVectorImpl<char> &CompressedContents,
-                             bool ZLibStyle, unsigned Alignment);
+                             bool ZLibStyle, bool ZstdStyle, unsigned Alignment);
 
----------------
Having two variables is ugly. Just pass `MAI->compressDebugSections()` as an argument.


================
Comment at: llvm/lib/MC/ELFObjectWriter.cpp:884
+  bool ZstdStyle = MAI->compressDebugSections() == DebugCompressionType::Zstd;
+  if(ZstdStyle){
+      compression::zstd::compress(StringRef(UncompressedData.data(), UncompressedData.size()),
----------------
`if (ZstdStyle)`


================
Comment at: llvm/lib/ObjCopy/ELF/ELFObject.cpp:565
+      CompressedData);
+  }else{
+    compression::zlib::compress(
----------------
clang-format


================
Comment at: llvm/lib/Object/Decompressor.cpp:24
+  DebugCompressionType CType = DebugCompressionType::Z;
+  if(isGnuStyle(Name)){
+    CType = DebugCompressionType::GNU;
----------------
clang-format

Please check the prevailing style first.


================
Comment at: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp:744
     }
-    if (!compression::elf::isAvailable())
-      return createStringError(
-          errc::invalid_argument,
-          "LLVM was not compiled with LLVM_ENABLE_ZLIB: can not compress");
+    if(Config.CompressionType == DebugCompressionType::Zstd) {
+      if (!compression::zstd::isAvailable())
----------------
clang-format


================
Comment at: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp:1014
         "LLVM was not compiled with LLVM_ENABLE_ZLIB: cannot decompress");
+}else{
+  if (Config.DecompressDebugSections && !compression::zstd::isAvailable())
----------------
clang-format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128667/new/

https://reviews.llvm.org/D128667