[LLD][ELF] - Make compression level be dependent on -On.

[grimar]

Summary

Currently LLD always use zlib compression level 6.
This patch changes it to use 1 for -O0, -O1 and 6 for -O2.

It fixes https://bugs.llvm.org/show_bug.cgi?id=44089.

There was also a thread in llvm-dev on this topic:
https://lists.llvm.org/pipermail/llvm-dev/2018-August/125020.html

Here is a table with results of building clang mentioned there:

Level   Time            Size
0       0m17.128s       2045081496   Z_NO_COMPRESSION
1       0m31.471s       922618584    Z_BEST_SPEED
2       0m32.659s       903642376
3       0m36.749s       890805856
4       0m41.532s       876697184
5       0m48.383s       862778576
6       1m3.176s        855251640    Z_DEFAULT_COMPRESSION
7       1m15.335s       853755920
8       2m0.561s        852497560
9       2m33.972s       852397408    Z_BEST_COMPRESSION

It shows that it is probably not reasonable to use values greater than 6.

[grimar]

• Rename test case: *.s -> *.test

[peter.smith]

Looks good to me, looks like it matches the discussion in https://bugs.llvm.org/show_bug.cgi?id=44089 will be worth waiting for US timezone based reviewers to comment.

[MaskRay]

lld/test/ELF/compressed-debug-level.test

5 ↗

(On Diff #230848)

For long linker options, prefer -- to - (with a few exceptions like -Ttext)

We can probably use cmp %t.default %t.O1 for this test.

[MaskRay]

Probably attach the table in the commit message:

Level   Time            Size
0       0m17.128s       2045081496   Z_NO_COMPRESSION
1       0m31.471s       922618584    Z_BEST_SPEED
2       0m32.659s       903642376
3       0m36.749s       890805856
4       0m41.532s       876697184
5       0m48.383s       862778576
6       1m3.176s        855251640    Z_DEFAULT_COMPRESSION
7       1m15.335s       853755920
8       2m0.561s        852497560
9       2m33.972s       852397408    Z_BEST_COMPRESSION

[MaskRay]

https://bugs.llvm.org/show_bug.cgi?id=44089

So, I think we should use level 1 for the default and level 6 for -O2, and I don't think we need a new option to specify some other value as a compression level.

-O0 => level 1
-O1 => level 1
-O2 => level 6

makes sense.

We use -Wl,--compress-debug-sections=zlib and default -Wl,-O1 for asan builds, so some users may observe size increases (due to: zlib level 6 -> zlib level 1). I will try accommodating the targets not to rely on the size, but I may request for a new option for compression level (@ruiu) if fixing them is not feasible. (I genuinely want to keep the configuration simple, so not having an additional option is also my preference.)

[ruiu]

LGTM

[ruiu]

lld/ELF/OutputSections.cpp

273

It's worth adding a comment here to explain why we chose 6 and 1 as optimization levels. We chose 1 as the default compression level because it is the fastest. If -O2 is given, we use level 6 to compress debug info more by ~15%. We found that level 7 to 9 doesn't make much difference (~1% more compression) while they take significant amount of time (~2x), so level 6 seems enough.

[grimar]

I've applied all the changes suggested in comments before committing. Thanks!

[smeenai]

lld/test/ELF/compressed-debug-level.test

23 ↗

(On Diff #231013)

The size appears to depend on the zlib version. For example, I'm running with zlib 1.2.8, which manages to compress these contents to the same size with both level 1 and 6. Is there any way to make this agnostic to zlib version?

[MaskRay]

lld/test/ELF/compressed-debug-level.test

23 ↗

(On Diff #231013)

I run zlib 1.2.11 and it works for me. Can you alter the input a bit to make the length different for level 1 and level 6, for zlib 1.2.8?

[smeenai]

lld/test/ELF/compressed-debug-level.test

23 ↗

(On Diff #231013)

I fixed it with 75fd939bb917e8f843395684a2970d86bc0199c0.

[grimar]

lld/test/ELF/compressed-debug-level.test

23 ↗

(On Diff #231013)

I fixed it with 75fd939bb917e8f843395684a2970d86bc0199c0.

Thanks! FTR, I used zlib 1.2.3 under windows when wrote this. Now I wonder how much brittle to check the size produced.
Seems it is not safe enough?
I think what we can do instead is to print llvm-readelf output to files for pairs [-O0, -O2], [-O1, -O2] etc and then do something
like

# CHECK: [ 1] .debug_info PROGBITS 00000000 000094 [[SIZE:*]]
# CHECK-NOT: [ 1] .debug_info PROGBITS 00000000 000094 [[SIZE]]

i.e. this should check that sized are just different. Should we?

[smeenai]

lld/test/ELF/compressed-debug-level.test

23 ↗

(On Diff #231013)

The issue I ran into with the original test case was that the compressed data had the same size with levels 1 and 6, so just verifying the sizes were different wouldn't help.

I agree that checking sizes isn't super robust, but it seems to work okay for now with the updated test case?

[MaskRay]

lld/test/ELF/compressed-debug-level.test

23 ↗

(On Diff #231013)

We could use not cmp %t.0 %t.2, but since it seems to work for zlib 1.2.3, 1.2.8, 1.2.11, probably not worth updating it again.

lld/ELF/OutputSections.cpp

template <class ELFT> void OutputSection::maybeCompress() {
   // Create a section header.
   zDebugHeader.resize(sizeof(Elf_Chdr));
   auto *hdr = reinterpret_cast<Elf_Chdr *>(zDebugHeader.data());
   hdr->ch_type = ELFCOMPRESS_ZLIB;
   hdr->ch_size = size;
   hdr->ch_addralign = alignment;
 
   // Write section contents to a temporary buffer and compress it.
   std::vector<uint8_t> buf(size);
   writeTo<ELFT>(buf.data());
-  if (Error e = zlib::compress(toStringRef(buf), compressedData))
+  if (Error e = zlib::compress(toStringRef(buf), compressedData,
+                               config->optimize >= 2 ? 6 : 1))
     fatal("compress failed: " + llvm::toString(std::move(e)));
 
   // Update section headers.
   size = sizeof(Elf_Chdr) + compressedData.size();
   flags |= SHF_COMPRESSED;
 }
 
 static void writeInt(uint8_t *buf, uint64_t data, uint64_t size) {

lld/test/ELF/compressed-debug-level.s

+# REQUIRES: x86, zlib
+
+# RUN: yaml2obj %s -o %t.o
+
+# RUN: ld.lld %t.o -o %t.default -compress-debug-sections=zlib
+# RUN: llvm-readelf --sections %t.default | FileCheck -check-prefixes=HEADER,LEVEL1 %s
+
+# RUN: ld.lld -O0 %t.o -o %t.O0 -compress-debug-sections=zlib
+# RUN: llvm-readelf --sections %t.O0 | FileCheck -check-prefixes=HEADER,LEVEL1 %s
+
+# RUN: ld.lld -O1 %t.o -o %t.O1 -compress-debug-sections=zlib
+# RUN: llvm-readelf --sections %t.O1 | FileCheck -check-prefixes=HEADER,LEVEL1 %s
+
+# RUN: ld.lld -O2 %t.o -o %t.O2 -compress-debug-sections=zlib
+# RUN: llvm-readelf --sections %t.O2 | FileCheck -check-prefixes=HEADER,LEVEL6 %s
+
+## LLD uses zlib compression of level 1 when -O0, -O1 and level 6 when -O2.
+## Here we check how -O flag affects the size of compressed sections produced.
+
+# HEADER: [Nr] Name        Type     Address  Off    Size
+# LEVEL1: [ 1] .debug_info PROGBITS 00000000 000094 00001c
+# LEVEL6: [ 1] .debug_info PROGBITS 00000000 000094 00001b
+
+## A little arbitrary debug section which has a different size after
+## applying compression of level 1 and 6.
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_386
+Sections:
+  - Name:    .debug_info
+    Type:    SHT_PROGBITS
+    Content: '01010201020201020102'