[PATCH] D31941: [ELF] - Implemented --compress-debug-sections option.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/Options.td:26
+def compress_debug_sections : J<"compress-debug-sections=">,
+  HelpText<"Compresses DWARF debug sections">;
+
----------------
ruiu wrote:
> Compresses -> Compress
Done.


================
Comment at: ELF/OutputSections.cpp:90
+// https://docs.oracle.com/cd/E53394_01/html/E54813/section_compression.html
+static std::vector<uint8_t> createHeader(size_t Size, uint32_t Alignment) {
+  llvm::support::endianness E = Config->Endianness;
----------------
ruiu wrote:
> I think it is better to template this function with ELFT so that you can access members by name using ELFT::Chdr type.
Done.


================
Comment at: ELF/OutputSections.cpp:115
+
+template <class ELFT> void OutputSection::compress() {
+  if (!(this->Flags & SHF_COMPRESSED))
----------------
ruiu wrote:
> Rename this `maybeComrpess` as it does't always compress contents.
Done.


================
Comment at: ELF/OutputSections.cpp:143-147
+  // If -compress-debug-sections is specified, we compress output debug
+  // sections.
+  if (Config->CompressDebugSections && Name.startswith(".debug_"))
+    if (!(Flags & SHF_ALLOC))
+      this->Flags |= SHF_COMPRESSED;
----------------
ruiu wrote:
> Move this to OutputSection::compress().
Done.


================
Comment at: ELF/OutputSections.h:92-93
+  // Used for implementation of --compress-debug-sections option.
+  llvm::SmallVector<char, 1> CompressedData;
+  std::vector<uint8_t> CompressedHeader;
+
----------------
ruiu wrote:
> CompressedHeader can be part of CompressedData.
That what I wanted to do, but I think there is no way to do that in effective way with current zlib::compress implementation.

It takes buffer for output, resizes and fills on its side. There is no way to say: "I need some room at start".
As a result after doing compression if you want to add some header all section content should be shifted and it can affect perfomance if section is huge. 

So I did them as separate fields for now.


================
Comment at: ELF/OutputSections.h:95-96
+
+  // Size of decompressed data. Only has meaning if compression enabled.
+  size_t DecompressedSize;
+
----------------
ruiu wrote:
> You don't need this, I guess?
Right. Removed.


https://reviews.llvm.org/D31941