public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Martin Liška" <mliska@suse.cz>
To: Fangrui Song <maskray@google.com>, Andrew Pinski <pinskia@gmail.com>
Cc: Fangrui Song <i@maskray.me>,
	GCC Patches <gcc-patches@gcc.gnu.org>,
	Richard Biener <richard.guenther@gmail.com>
Subject: Re: [PATCH] Remove legacy -gz=zlib-gnu
Date: Tue, 20 Sep 2022 14:55:02 +0200	[thread overview]
Message-ID: <ab1530b2-bfc8-903f-ace4-97788b145ba7@suse.cz> (raw)
In-Reply-To: <20220701072057.f2ojdm4pdm5257ni@google.com>

On 7/1/22 09:20, Fangrui Song via Gcc-patches wrote:
> On 2022-07-01, Andrew Pinski wrote:
>> On Thu, Jun 30, 2022 at 11:58 PM Fangrui Song via Gcc-patches
>> <gcc-patches@gcc.gnu.org> wrote:
>>>
>>> From: Fangrui Song <i@maskray.me>
>>>
>>> SHF_COMPRESSED style zlib has been supported since binutils 2.26
>>> and the legacy zlib-gnu option hasn't gain adoption.
>>> According to Debian Code Search (`gz=zlib-gnu`), no project uses
>>> -gz=zlib-gnu (valgrind has a configure to use -gz=zlib).
>>> Remove support for the legacy zlib-gnu and simplify configure.ac by
>>> removing zlib-gnu ld/as check.
>>
>> A couple of things, you are missing a changelog.
> 
> Sorry.
> 
>> Second, why remove something which is still working?

Hi.

I do support the option removal, while I would replace the removal with a warning
saying no compression will be used.

> 
> It's unused and its existence causes confusion: the paradox of choice.
> People may assume the support may be good but newer DWARF consumers may
> not support the legacy format.

Agree, the compression format is legacy. I verified all openSUSE packages (15k)
and there's no project actively using it.

> 
> The other motivation is to clean up it a bit.  I foresee that someone
> will add --compress-debug-sections=zstd to binutils and configure.ac and
> gcc/gcc.cc would become more messy.

The argument makes sense, it will be even bigger mess.

@Richi: Is it something we can deprecate for GCC 13?

Martin

> 
>> Third, why not just make gz=zlib-gnu as an alias to gz=zlib instead so
>> if someone used it before it will still work. we try not to remove
>> options; have them emit a warning and be ignored (or moved over to the
>> closed option).
> 
> Changing the semantics of -gz=zlib-gnu would be even more confusing.
> 
>> Thanks,
>> Andrew
>>
>>> ---
>>>  gcc/common.opt      |  3 ---
>>>  gcc/configure       | 33 ++++++---------------------------
>>>  gcc/configure.ac    | 29 ++++-------------------------
>>>  gcc/doc/invoke.texi | 11 +++++------
>>>  gcc/gcc.cc          | 22 ++--------------------
>>>  5 files changed, 17 insertions(+), 81 deletions(-)
>>>
>>> diff --git a/gcc/common.opt b/gcc/common.opt
>>> index e7a51e882ba..8754d93d545 100644
>>> --- a/gcc/common.opt
>>> +++ b/gcc/common.opt
>>> @@ -3424,9 +3424,6 @@ Enum(compressed_debug_sections) String(none) Value(0)
>>>  EnumValue
>>>  Enum(compressed_debug_sections) String(zlib) Value(1)
>>>
>>> -EnumValue
>>> -Enum(compressed_debug_sections) String(zlib-gnu) Value(2)
>>> -
>>>  gz
>>>  Common Driver
>>>  Generate compressed debug sections.
>>> diff --git a/gcc/configure b/gcc/configure
>>> index 62872d132ea..ca87e875e9d 100755
>>> --- a/gcc/configure
>>> +++ b/gcc/configure
>>> @@ -19674,7 +19674,7 @@ else
>>>    lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>>>    lt_status=$lt_dlunknown
>>>    cat > conftest.$ac_ext <<_LT_EOF
>>> -#line 19679 "configure"
>>> +#line 19677 "configure"
>>>  #include "confdefs.h"
>>>
>>>  #if HAVE_DLFCN_H
>>> @@ -19780,7 +19780,7 @@ else
>>>    lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>>>    lt_status=$lt_dlunknown
>>>    cat > conftest.$ac_ext <<_LT_EOF
>>> -#line 19785 "configure"
>>> +#line 19783 "configure"
>>>  #include "confdefs.h"
>>>
>>>  #if HAVE_DLFCN_H
>>> @@ -29711,20 +29711,13 @@ else
>>>     if $gcc_cv_as --compress-debug-sections -o conftest.o conftest.s 2>&1 | grep -i warning > /dev/null
>>>     then
>>>       gcc_cv_as_compress_debug=0
>>> -   # Since binutils 2.26, gas supports --compress-debug-sections=type,
>>> +   # Since binutils 2.26, gas supports --compress-debug-sections=zlib,
>>>     # defaulting to the ELF gABI format.
>>> -   elif $gcc_cv_as --compress-debug-sections=zlib-gnu -o conftest.o conftest.s > /dev/null 2>&1
>>> +   elif $gcc_cv_as --compress-debug-sections=zlib -o conftest.o conftest.s > /dev/null 2>&1
>>>     then
>>>       gcc_cv_as_compress_debug=2
>>>       gcc_cv_as_compress_debug_option="--compress-debug-sections"
>>>       gcc_cv_as_no_compress_debug_option="--nocompress-debug-sections"
>>> -   # Before binutils 2.26, gas only supported --compress-debug-options and
>>> -   # emitted the traditional GNU format.
>>> -   elif $gcc_cv_as --compress-debug-sections -o conftest.o conftest.s > /dev/null 2>&1
>>> -   then
>>> -     gcc_cv_as_compress_debug=1
>>> -     gcc_cv_as_compress_debug_option="--compress-debug-sections"
>>> -     gcc_cv_as_no_compress_debug_option="--nocompress-debug-sections"
>>>     else
>>>       gcc_cv_as_compress_debug=0
>>>     fi
>>> @@ -30238,42 +30231,28 @@ $as_echo "$gcc_cv_ld_eh_gc_sections_bug" >&6; }
>>>
>>>  { $as_echo "$as_me:${as_lineno-$LINENO}: checking linker for compressed debug sections" >&5
>>>  $as_echo_n "checking linker for compressed debug sections... " >&6; }
>>> -# gold/gld support compressed debug sections since binutils 2.19/2.21
>>> -# In binutils 2.26, gld gained support for the ELF gABI format.
>>> +# GNU ld/gold support --compressed-debug-sections=zlib since binutils 2.26.
>>>  if test $in_tree_ld = yes ; then
>>>    gcc_cv_ld_compress_debug=0
>>>    if test $ld_is_mold = yes; then
>>>      gcc_cv_ld_compress_debug=3
>>>      gcc_cv_ld_compress_debug_option="--compress-debug-sections"
>>> -  elif test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 19 -o "$gcc_cv_gld_major_version" -gt 2 \
>>> -     && test $in_tree_ld_is_elf = yes && test $ld_is_gold = yes; then
>>> -    gcc_cv_ld_compress_debug=2
>>> -    gcc_cv_ld_compress_debug_option="--compress-debug-sections"
>>>    elif test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 26 -o "$gcc_cv_gld_major_version" -gt 2 \
>>>       && test $in_tree_ld_is_elf = yes && test $ld_is_gold = no; then
>>>      gcc_cv_ld_compress_debug=3
>>>      gcc_cv_ld_compress_debug_option="--compress-debug-sections"
>>> -  elif test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 21 -o "$gcc_cv_gld_major_version" -gt 2 \
>>> -     && test $in_tree_ld_is_elf = yes; then
>>> -    gcc_cv_ld_compress_debug=1
>>>    fi
>>>  elif echo "$ld_ver" | grep GNU > /dev/null; then
>>>    if test $ld_is_mold = yes; then
>>>      gcc_cv_ld_compress_debug=3
>>>      gcc_cv_ld_compress_debug_option="--compress-debug-sections"
>>>    elif test "$ld_vers_major" -lt 2 \
>>> -     || test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -lt 21; then
>>> +     || test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -lt 26; then
>>>      gcc_cv_ld_compress_debug=0
>>> -  elif test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -lt 26; then
>>> -    gcc_cv_ld_compress_debug=1
>>>    else
>>>      gcc_cv_ld_compress_debug=3
>>>      gcc_cv_ld_compress_debug_option="--compress-debug-sections"
>>>    fi
>>> -  if test $ld_is_gold = yes; then
>>> -    gcc_cv_ld_compress_debug=2
>>> -    gcc_cv_ld_compress_debug_option="--compress-debug-sections"
>>> -  fi
>>>  else
>>>    case "${target}" in
>>>      *-*-solaris2*)
>>> diff --git a/gcc/configure.ac b/gcc/configure.ac
>>> index 446747311a6..f40f75271c2 100644
>>> --- a/gcc/configure.ac
>>> +++ b/gcc/configure.ac
>>> @@ -5733,20 +5733,13 @@ gcc_GAS_CHECK_FEATURE([compressed debug sections],
>>>     if $gcc_cv_as --compress-debug-sections -o conftest.o conftest.s 2>&1 | grep -i warning > /dev/null
>>>     then
>>>       gcc_cv_as_compress_debug=0
>>> -   # Since binutils 2.26, gas supports --compress-debug-sections=type,
>>> +   # Since binutils 2.26, gas supports --compress-debug-sections=zlib,
>>>     # defaulting to the ELF gABI format.
>>> -   elif $gcc_cv_as --compress-debug-sections=zlib-gnu -o conftest.o conftest.s > /dev/null 2>&1
>>> +   elif $gcc_cv_as --compress-debug-sections=zlib -o conftest.o conftest.s > /dev/null 2>&1
>>>     then
>>>       gcc_cv_as_compress_debug=2
>>>       gcc_cv_as_compress_debug_option="--compress-debug-sections"
>>>       gcc_cv_as_no_compress_debug_option="--nocompress-debug-sections"
>>> -   # Before binutils 2.26, gas only supported --compress-debug-options and
>>> -   # emitted the traditional GNU format.
>>> -   elif $gcc_cv_as --compress-debug-sections -o conftest.o conftest.s > /dev/null 2>&1
>>> -   then
>>> -     gcc_cv_as_compress_debug=1
>>> -     gcc_cv_as_compress_debug_option="--compress-debug-sections"
>>> -     gcc_cv_as_no_compress_debug_option="--nocompress-debug-sections"
>>>     else
>>>       gcc_cv_as_compress_debug=0
>>>     fi])
>>> @@ -6131,42 +6124,28 @@ fi
>>>  AC_MSG_RESULT($gcc_cv_ld_eh_gc_sections_bug)
>>>
>>>  AC_MSG_CHECKING(linker for compressed debug sections)
>>> -# gold/gld support compressed debug sections since binutils 2.19/2.21
>>> -# In binutils 2.26, gld gained support for the ELF gABI format.
>>> +# GNU ld/gold support --compressed-debug-sections=zlib since binutils 2.26.
>>>  if test $in_tree_ld = yes ; then
>>>    gcc_cv_ld_compress_debug=0
>>>    if test $ld_is_mold = yes; then
>>>      gcc_cv_ld_compress_debug=3
>>>      gcc_cv_ld_compress_debug_option="--compress-debug-sections"
>>> -  elif test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 19 -o "$gcc_cv_gld_major_version" -gt 2 \
>>> -     && test $in_tree_ld_is_elf = yes && test $ld_is_gold = yes; then
>>> -    gcc_cv_ld_compress_debug=2
>>> -    gcc_cv_ld_compress_debug_option="--compress-debug-sections"
>>>    elif test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 26 -o "$gcc_cv_gld_major_version" -gt 2 \
>>>       && test $in_tree_ld_is_elf = yes && test $ld_is_gold = no; then
>>>      gcc_cv_ld_compress_debug=3
>>>      gcc_cv_ld_compress_debug_option="--compress-debug-sections"
>>> -  elif test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 21 -o "$gcc_cv_gld_major_version" -gt 2 \
>>> -     && test $in_tree_ld_is_elf = yes; then
>>> -    gcc_cv_ld_compress_debug=1
>>>    fi
>>>  elif echo "$ld_ver" | grep GNU > /dev/null; then
>>>    if test $ld_is_mold = yes; then
>>>      gcc_cv_ld_compress_debug=3
>>>      gcc_cv_ld_compress_debug_option="--compress-debug-sections"
>>>    elif test "$ld_vers_major" -lt 2 \
>>> -     || test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -lt 21; then
>>> +     || test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -lt 26; then
>>>      gcc_cv_ld_compress_debug=0
>>> -  elif test "$ld_vers_major" -eq 2 -a "$ld_vers_minor" -lt 26; then
>>> -    gcc_cv_ld_compress_debug=1
>>>    else
>>>      gcc_cv_ld_compress_debug=3
>>>      gcc_cv_ld_compress_debug_option="--compress-debug-sections"
>>>    fi
>>> -  if test $ld_is_gold = yes; then
>>> -    gcc_cv_ld_compress_debug=2
>>> -    gcc_cv_ld_compress_debug_option="--compress-debug-sections"
>>> -  fi
>>>  else
>>>  changequote(,)dnl
>>>    case "${target}" in
>>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>>> index 757775ea576..467659cb094 100644
>>> --- a/gcc/doc/invoke.texi
>>> +++ b/gcc/doc/invoke.texi
>>> @@ -10775,12 +10775,11 @@ location views are enabled.
>>>  Produce compressed debug sections in DWARF format, if that is supported.
>>>  If @var{type} is not given, the default type depends on the capabilities
>>>  of the assembler and linker used.  @var{type} may be one of
>>> -@samp{none} (don't compress debug sections), @samp{zlib} (use zlib
>>> -compression in ELF gABI format), or @samp{zlib-gnu} (use zlib
>>> -compression in traditional GNU format).  If the linker doesn't support
>>> -writing compressed debug sections, the option is rejected.  Otherwise,
>>> -if the assembler does not support them, @option{-gz} is silently ignored
>>> -when producing object files.
>>> +@samp{none} (don't compress debug sections), or @samp{zlib} (use zlib
>>> +compression in ELF gABI format).  If the linker doesn't support writing
>>> +compressed debug sections, the option is rejected.  Otherwise, if the
>>> +assembler does not support them, @option{-gz} is silently ignored when
>>> +producing object files.
>>>
>>>  @item -femit-struct-debug-baseonly
>>>  @opindex femit-struct-debug-baseonly
>>> diff --git a/gcc/gcc.cc b/gcc/gcc.cc
>>> index 5cbb38560b2..660cd6f4ef4 100644
>>> --- a/gcc/gcc.cc
>>> +++ b/gcc/gcc.cc
>>> @@ -827,22 +827,11 @@ proper position among the other output files.  */
>>>  /* No linker support.  */
>>>  #define LINK_COMPRESS_DEBUG_SPEC \
>>>         " %{gz*:%e-gz is not supported in this configuration} "
>>> -#elif HAVE_LD_COMPRESS_DEBUG == 1
>>> -/* GNU style on input, GNU ld options.  Reject, not useful.  */
>>> -#define LINK_COMPRESS_DEBUG_SPEC \
>>> -       " %{gz*:%e-gz is not supported in this configuration} "
>>> -#elif HAVE_LD_COMPRESS_DEBUG == 2
>>> -/* GNU style, GNU gold options.  */
>>> -#define LINK_COMPRESS_DEBUG_SPEC \
>>> -       " %{gz|gz=zlib-gnu:" LD_COMPRESS_DEBUG_OPTION "=zlib}" \
>>> -       " %{gz=none:"        LD_COMPRESS_DEBUG_OPTION "=none}" \
>>> -       " %{gz=zlib:%e-gz=zlib is not supported in this configuration} "
>>>  #elif HAVE_LD_COMPRESS_DEBUG == 3
>>>  /* ELF gABI style.  */
>>>  #define LINK_COMPRESS_DEBUG_SPEC \
>>>         " %{gz|gz=zlib:"  LD_COMPRESS_DEBUG_OPTION "=zlib}" \
>>> -       " %{gz=none:"     LD_COMPRESS_DEBUG_OPTION "=none}" \
>>> -       " %{gz=zlib-gnu:" LD_COMPRESS_DEBUG_OPTION "=zlib-gnu} "
>>> +       " %{gz=none:"     LD_COMPRESS_DEBUG_OPTION "=none}"
>>>  #else
>>>  #error Unknown value for HAVE_LD_COMPRESS_DEBUG.
>>>  #endif
>>> @@ -891,18 +880,11 @@ proper position among the other output files.  */
>>>  /* No assembler support.  Ignore silently.  */
>>>  #define ASM_COMPRESS_DEBUG_SPEC \
>>>         " %{gz*:} "
>>> -#elif HAVE_AS_COMPRESS_DEBUG == 1
>>> -/* GNU style, GNU as options.  */
>>> -#define ASM_COMPRESS_DEBUG_SPEC \
>>> -       " %{gz|gz=zlib-gnu:" AS_COMPRESS_DEBUG_OPTION "}" \
>>> -       " %{gz=none:"        AS_NO_COMPRESS_DEBUG_OPTION "}" \
>>> -       " %{gz=zlib:%e-gz=zlib is not supported in this configuration} "
>>>  #elif HAVE_AS_COMPRESS_DEBUG == 2
>>>  /* ELF gABI style.  */
>>>  #define ASM_COMPRESS_DEBUG_SPEC \
>>>         " %{gz|gz=zlib:"  AS_COMPRESS_DEBUG_OPTION "=zlib}" \
>>> -       " %{gz=none:"     AS_COMPRESS_DEBUG_OPTION "=none}" \
>>> -       " %{gz=zlib-gnu:" AS_COMPRESS_DEBUG_OPTION "=zlib-gnu} "
>>> +       " %{gz=none:"     AS_COMPRESS_DEBUG_OPTION "=none}"
>>>  #else
>>>  #error Unknown value for HAVE_AS_COMPRESS_DEBUG.
>>>  #endif
>>> -- 
>>> 2.37.0.rc0.161.g10f37bed90-goog
>>>


  reply	other threads:[~2022-09-20 12:55 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-01  6:57 Fangrui Song
2022-07-01  7:03 ` Andrew Pinski
2022-07-01  7:20   ` Fangrui Song
2022-09-20 12:55     ` Martin Liška [this message]
2022-09-21  7:36       ` Richard Biener
2022-09-21  7:45         ` Fangrui Song
2022-09-21  7:49         ` Martin Liška
2022-09-21  9:35           ` Richard Biener
2022-09-22 11:10             ` [PATCH] remove -gz=zlib-gnu option value Martin Liška
2022-09-22 12:26               ` [PATCH v2] " Martin Liška
2022-09-22 12:35                 ` Richard Biener
2022-09-22 12:51                   ` [PATCH] support -gz=zstd for both linker and assembler Martin Liška
2022-09-27 13:54                     ` Martin Liška
2022-09-28 19:21                       ` Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ab1530b2-bfc8-903f-ace4-97788b145ba7@suse.cz \
    --to=mliska@suse.cz \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=i@maskray.me \
    --cc=maskray@google.com \
    --cc=pinskia@gmail.com \
    --cc=richard.guenther@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).