[Driver] Remove obsoleted -gz=zlib-gnu

[MaskRay]

Summary

GCC added -gz=zlib-gnu in 2014 for -gz meaning change (.zdebug =>
SHF_COMPRESSED) and the legacy zlib-gnu hasn't gain adoption.

According to Debian Code Search, no project uses -gz=zlib-gnu (valgrind has a
configure to use -gz=zlib). Any possible -gz=zlib-gnu user can switch to -gz
smoothly (supported by integrated assemblers for many years; binutils 2.26).

[MaskRay]

fix a comment

[dblaikie]

According to Debian Code Search, no project uses -gz=zlib-gnu (valgrind has a configure to use -gz=zlib)

Could you link to the queries you used to validate this? I'd be curious to take a look around.

[MaskRay]

In D117744#3259305, @dblaikie wrote:

According to Debian Code Search, no project uses -gz=zlib-gnu (valgrind has a configure to use -gz=zlib)

Could you link to the queries you used to validate this? I'd be curious to take a look around.

https://codesearch.debian.net/search?q=gz%3Dzlib-gnu&literal=0

[MaskRay]

Ping:)

[dblaikie]

I don't especially feel that removing low-cost features like this is a great idea (the benefit to cleaning up a small amount of code compared to the (albeit small) risk of breaking users doesn't seem high value to me), but sure - give it a go.

[MaskRay]

In D117744#3273463, @dblaikie wrote:

I don't especially feel that removing low-cost features like this is a great idea (the benefit to cleaning up a small amount of code compared to the (albeit small) risk of breaking users doesn't seem high value to me), but sure - give it a go.

Thanks:)

This is to ensure that there are no producers: then we can do something more ambitious, like removing assembler support:)

Driver change is a good start because if someone has justifiable complaint, the change can be relatively easily reversed without churn to the underlying implementation.

It will also make future removal of .zdebug from ld.lld justifiable.

[dblaikie]

In D117744#3273481, @MaskRay wrote:

In D117744#3273463, @dblaikie wrote:

I don't especially feel that removing low-cost features like this is a great idea (the benefit to cleaning up a small amount of code compared to the (albeit small) risk of breaking users doesn't seem high value to me), but sure - give it a go.

Thanks:)

This is to ensure that there are no producers: then we can do something more ambitious, like removing assembler support:)

Driver change is a good start because if someone has justifiable complaint, the change can be relatively easily reversed without churn to the underlying implementation.

It will also make future removal of .zdebug from ld.lld justifiable.

Yeah, I appreciate the desire, but it doesn't seem especially burdensome to support (since we'll still have support for the SHF_COMPRESSED style and it's not a lot of code on top of that to support the zdebug style) so I don't see as much value in making these changes, but that's OK.

Oh, one thing: would probably be good to make two separate commits, one for -gz and one for the --compress-debug-sections

[MaskRay]

In D117744#3273521, @dblaikie wrote:

In D117744#3273481, @MaskRay wrote:

In D117744#3273463, @dblaikie wrote:

I don't especially feel that removing low-cost features like this is a great idea (the benefit to cleaning up a small amount of code compared to the (albeit small) risk of breaking users doesn't seem high value to me), but sure - give it a go.

Thanks:)

This is to ensure that there are no producers: then we can do something more ambitious, like removing assembler support:)

Driver change is a good start because if someone has justifiable complaint, the change can be relatively easily reversed without churn to the underlying implementation.

It will also make future removal of .zdebug from ld.lld justifiable.

Yeah, I appreciate the desire, but it doesn't seem especially burdensome to support (since we'll still have support for the SHF_COMPRESSED style and it's not a lot of code on top of that to support the zdebug style) so I don't see as much value in making these changes, but that's OK.

The issue with .zdebug is the section name. When iterating over input sections (which may cost ld.lld 0.4% time, the ratio will go up if we support parallel symbol table initialization), not needing to scan section names (different places from the section metadata) will improve a bit.
Yeah, it's saving on some very minor stuff, I know.

Oh, one thing: would probably be good to make two separate commits, one for -gz and one for the --compress-debug-sections

OK

[labath]

This (unsurprisingly) broke an lldb test for the gnu style compression. You didn't get the notification because the bot was already red at the time. I've already fixed the situation by converting the test to yaml, but I thought you may want to know what happened.

[MaskRay]

In D117744#3275226, @labath wrote:

This (unsurprisingly) broke an lldb test for the gnu style compression. You didn't get the notification because the bot was already red at the time. I've already fixed the situation by converting the test to yaml, but I thought you may want to know what happened.

Thanks for aaa9f40e3fd2dd081e965d39f10649bcd8bf1d21 :) Sorry that I missed -Wa,--compress-debug-sections=zlib-gnu in the test.

To be accurate, the zlib-gnu style compression is still available with llvm-mc -filetype=obj -compress-debug-sections=zlib-gnu (that I haven't dropped but will do after a while).
It's just that the producer feature is not expoed in any user-facing tool (clang, lld).

And sound good to remove lldb reading support at some point.
I have found that Go's own linker generates .zdebug and notified them to fix it (https://go-review.googlesource.com/c/go/+/380755/)
If lldb debugability with older Go (once they have fixed the issue) is not so useful, yeah, drop the support.

clang/include/clang/Driver/Options.td

def debugger_tuning_EQ : Joined<["-"], "debugger-tuning=">,
   MarshallingInfoEnum<CodeGenOpts<"DebuggerTuning">, "Default">;
 def dwarf_debug_flags : Separate<["-"], "dwarf-debug-flags">,
   HelpText<"The string to embed in the Dwarf debug flags record.">,
   MarshallingInfoString<CodeGenOpts<"DwarfDebugFlags">>;
 def record_command_line : Separate<["-"], "record-command-line">,
   HelpText<"The string to embed in the .LLVM.command.line section.">,
   MarshallingInfoString<CodeGenOpts<"RecordCommandLine">>;
 def compress_debug_sections_EQ : Joined<["-", "--"], "compress-debug-sections=">,
-    HelpText<"DWARF debug sections compression type">, Values<"none,zlib,zlib-gnu">,
-    NormalizedValuesScope<"llvm::DebugCompressionType">, NormalizedValues<["None", "Z", "GNU"]>,
+    HelpText<"DWARF debug sections compression type">, Values<"none,zlib">,
+    NormalizedValuesScope<"llvm::DebugCompressionType">, NormalizedValues<["None", "Z"]>,
     MarshallingInfoEnum<CodeGenOpts<"CompressDebugSections">, "None">;
 def compress_debug_sections : Flag<["-", "--"], "compress-debug-sections">,
   Alias<compress_debug_sections_EQ>, AliasArgs<["zlib"]>;
 def mno_exec_stack : Flag<["-"], "mnoexecstack">,
   HelpText<"Mark the file as not needing an executable stack">,
   MarshallingInfoFlag<CodeGenOpts<"NoExecStack">>;
 def massembler_no_warn : Flag<["-"], "massembler-no-warn">,
   HelpText<"Make assembler not emit warnings">,

clang/lib/Driver/ToolChains/Clang.cpp

static void RenderDebugInfoCompressionArgs(const ArgList &Args,
                                            const ToolChain &TC) {
   const Arg *A = Args.getLastArg(options::OPT_gz_EQ);
   if (!A)
     return;
   if (checkDebugInfoOption(A, Args, D, TC)) {
     StringRef Value = A->getValue();
     if (Value == "none") {
       CmdArgs.push_back("--compress-debug-sections=none");
-    } else if (Value == "zlib" || Value == "zlib-gnu") {
+    } else if (Value == "zlib") {
       if (llvm::zlib::isAvailable()) {
         CmdArgs.push_back(
             Args.MakeArgString("--compress-debug-sections=" + Twine(Value)));
       } else {
         D.Diag(diag::warn_debug_compression_unavailable);
       }
     } else {
       D.Diag(diag::err_drv_unsupported_option_argument)

clang/lib/Driver/ToolChains/CommonArgs.cpp

for (const auto &II : Inputs) {
     }
   }
 }
 
 void tools::addLinkerCompressDebugSectionsOption(
     const ToolChain &TC, const llvm::opt::ArgList &Args,
     llvm::opt::ArgStringList &CmdArgs) {
   // GNU ld supports --compress-debug-sections=none|zlib|zlib-gnu|zlib-gabi
-  // whereas zlib is an alias to zlib-gabi. Therefore -gz=none|zlib|zlib-gnu
-  // are translated to --compress-debug-sections=none|zlib|zlib-gnu.
-  // -gz is not translated since ld --compress-debug-sections option requires an
+  // whereas zlib is an alias to zlib-gabi and zlib-gnu is obsoleted. Therefore
+  // -gz=none|zlib are translated to --compress-debug-sections=none|zlib. -gz
+  // is not translated since ld --compress-debug-sections option requires an
   // argument.
   if (const Arg *A = Args.getLastArg(options::OPT_gz_EQ)) {
     StringRef V = A->getValue();
-    if (V == "none" || V == "zlib" || V == "zlib-gnu")
+    if (V == "none" || V == "zlib")
       CmdArgs.push_back(Args.MakeArgString("--compress-debug-sections=" + V));
     else
       TC.getDriver().Diag(diag::err_drv_unsupported_option_argument)
           << A->getOption().getName() << V;
   }
 }
 
 void tools::AddTargetFeature(const ArgList &Args,

clang/test/Driver/amdgcn-gz-options.cl

 // REQUIRES: zlib, amdgpu-registered-target
 
 // RUN: %clang -### -target amdgcn-amd-amdhsa -gz=none -x assembler %s 2>&1 | FileCheck -check-prefix CHECK-OPT_GZ_EQ_NONE %s
 // RUN: %clang -### -target amdgcn-amd-amdhsa -gz=none %s 2>&1 | FileCheck -check-prefix CHECK-OPT_GZ_EQ_NONE %s
 // CHECK-OPT_GZ_EQ_NONE: {{.* "-cc1(as)?".* "--compress-debug-sections=none"}}
 // CHECK-OPT_GZ_EQ_NONE: "--compress-debug-sections=none"
 
 // RUN: %clang -### -target amdgcn-amd-amdhsa -gz=zlib -x assembler %s 2>&1 | FileCheck -check-prefix CHECK-OPT_GZ_EQ_ZLIB %s
 // RUN: %clang -### -target amdgcn-amd-amdhsa -gz=zlib %s 2>&1 | FileCheck -check-prefix CHECK-OPT_GZ_EQ_ZLIB %s
 // CHECK-OPT_GZ_EQ_ZLIB: {{.* "-cc1(as)?".* "--compress-debug-sections=zlib"}}
 // CHECK-OPT_GZ_EQ_ZLIB: "--compress-debug-sections=zlib"
-
-// RUN: %clang -### -target amdgcn-amd-amdhsa -gz=zlib-gnu -x assembler %s 2>&1 | FileCheck -check-prefix CHECK-OPT_GZ_EQ_ZLIB_GNU %s
-// RUN: %clang -### -target amdgcn-amd-amdhsa -gz=zlib-gnu %s 2>&1 | FileCheck -check-prefix CHECK-OPT_GZ_EQ_ZLIB_GNU %s
-// CHECK-OPT_GZ_EQ_ZLIB_GNU: {{.* "-cc1(as)?".* "--compress-debug-sections=zlib-gnu"}}
-// CHECK-OPT_GZ_EQ_ZLIB_GNU: "--compress-debug-sections=zlib-gnu"

clang/test/Driver/compress.c

 // CHECK-OPT_GZ_EQ_NONE: "--compress-debug-sections=none"
 
 // RUN: %clang -### -target x86_64-unknown-linux-gnu -gz=zlib -x assembler %s 2>&1 | FileCheck -check-prefix CHECK-OPT_GZ_EQ_ZLIB %s
 // RUN: %clang -### -target x86_64-unknown-linux-gnu -gz=zlib %s 2>&1 | FileCheck -check-prefix CHECK-OPT_GZ_EQ_ZLIB %s
 // RUN: %clang -### -target x86_64-unknown-freebsd -gz=zlib %s 2>&1 | FileCheck -check-prefix CHECK-OPT_GZ_EQ_ZLIB %s
 // CHECK-OPT_GZ_EQ_ZLIB: {{.* "-cc1(as)?".* "--compress-debug-sections=zlib"}}
 // CHECK-OPT_GZ_EQ_ZLIB: "--compress-debug-sections=zlib"
 
-// RUN: %clang -### -target x86_64-unknown-linux-gnu -gz=zlib-gnu -x assembler %s 2>&1 | FileCheck -check-prefix CHECK-OPT_GZ_EQ_ZLIB_GNU %s
-// RUN: %clang -### -target x86_64-unknown-linux-gnu -gz=zlib-gnu %s 2>&1 | FileCheck -check-prefix CHECK-OPT_GZ_EQ_ZLIB_GNU %s
-// CHECK-OPT_GZ_EQ_ZLIB_GNU: {{.* "-cc1(as)?".* "--compress-debug-sections=zlib-gnu"}}
-// CHECK-OPT_GZ_EQ_ZLIB_GNU: "--compress-debug-sections=zlib-gnu"
-
 // RUN: %clang -### -fintegrated-as -gz=invalid -x assembler -c %s 2>&1 | FileCheck -check-prefix CHECK-OPT_GZ_EQ_INVALID %s
 // RUN: %clang -### -fintegrated-as -gz=invalid -c %s 2>&1 | FileCheck -check-prefix CHECK-OPT_GZ_EQ_INVALID %s
 // CHECK-OPT_GZ_EQ_INVALID: error: unsupported argument 'invalid' to option 'gz='