[PATCH] D130724: [MC] Support writing ELFCOMPRESS_ZSTD compressed debug info sections

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/MC/ELFObjectWriter.cpp:861-879
+  ArrayRef<uint8_t> Uncompressed =
       makeArrayRef(reinterpret_cast<uint8_t *>(UncompressedData.data()),
-                   UncompressedData.size()),
-      Compressed);
+                   UncompressedData.size());
+
+  SmallVector<uint8_t, 128> Compressed;
+  uint32_t ChType;
+  switch (MAI->compressDebugSections()) {
----------------
dblaikie wrote:
> Proliferation of these sort of switch statements is why I'd like to resolve the design discussions before further patches are committed. They're not hugely expensive to fix/change later, but neither are we in a rush to add this functionality, nor is the design discussion especially hard to resolve.
> Code something like:
> ```
> if (CompressionAlgorithm *C = getCompressionAlgorithm(MAI->compressDebugSections()) {
>   ChType = C->getELFType(); // alternatively have an array to lookup ELF type from the enum type
>   C->compress(Uncompressed, Compressed);
> }
> ```
> Seems like it'd be quite achievable without great complexity & simplify consumers.
> 
> Possibly even having 'compressDebugSections' carry the algorithm pointer directly? (since the checking's already done up-front, and the pointer would be to an immutable/thread safe/singleton/eternal object anyway)
I wish that 15.0.0 would have the llvm-objcopy change. This helps distributions which install llvm binary utilities in /usr/bin which may use newer userland llvm utilities.
MC/clang -gz support certainly can go into 16.0.0.

After this MC change, the only remaining place using `DebugCompressionType` is clang -gz. We don't have many ELF object producers, and the object-oriented `CompressionAlgorithm` design kinda makes me feel some over-engineering.

Note: in many (among the few `DebugCompressionType` uses) use cases `isAvailable` and `compress` are called apart.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130724