Re: [PATCH] gas: Add --force-compress-debug-sections

[Tom de Vries <tdevries@suse.de>]

On 2/27/23 15:07, Jan Beulich wrote:
> On 27.02.2023 14:44, Pedro Alves wrote:
>> On 2023-02-27 9:03 a.m., Jan Beulich via Binutils wrote:
>>> On 24.02.2023 15:57, Tom de Vries wrote:
>>>>
>>>> Is this more how you want it?
>>>
>>> I have to admit that I'm still puzzled by the presence of
>>> finalize_parse_compress_debug_optarg() as well as you needing both a new
>>> static variable and a new global one. But I guess whether that's really
>>> needed first of all depends on the semantics we want e.g.
>>>
>>> --nocompress-debug-sections --compress-debug-sections=force
>>>
>>> to have (which, with how you have it presently, could also be expressed
>>> as
>>>
>>> --compress-debug-sections=none+force
>>>
>>> or
>>>
>>> --compress-debug-sections=none --compress-debug-sections=force
>>>
>>> afaict). I view the present meaning as one sensible one, but I could
>>> also see "none" (or equivalent) simply zapping the compression type
>>> (and hence rendering "force" meaningless) as another sensible one. A
>>> change in meaning may then also result in the three option combinations
>>> above possibly not all doing the same.
>>
>>
>> ISTM that these confusions (which no doubt users will have too) would go away if
>> we did not try to put orthogonal settings into one option.
>>
>> Witness how the implementation uses two different enums:
>>
>> This new one:
>>
>>   +enum compress_debug_action
>>   +{
>>   +  cda_default,
>>   +  cda_none,
>>   +  cda_force,
>>   +  cda_yes,
>>   +};
>>
>> And this preexisting one:
>>
>>   /* Types of compressed DWARF debug sections.  */
>>   enum compressed_debug_section_type
>>   {
>>     COMPRESS_DEBUG_NONE = 0,
>>     COMPRESS_DEBUG_GNU_ZLIB = 1 << 1,
>>     COMPRESS_DEBUG_GABI_ZLIB = 1 << 2,
>>     COMPRESS_DEBUG_ZSTD = 1 << 3,
>>     COMPRESS_UNKNOWN = 1 << 4
>>   };
>>
>> Imagine we started over from scratch, and had these two orthogonal options,
>> matching internal enums (enum compress_debug_action would be slightly different):
>>
>>   --compressed-debug-sections-format=zlib|zstd|...|none
>>
>>     # Iff we're compressing, what format shall we use?
>>
>>   --compress-debug-sections=no|yes|sizewin
>>
>>     # Are we compressing debug sections?  When?
>>     #
>>     #  - "no" - never, we're not compressing.
>>     #
>>     #  - "yes" - always, we're compressing, using the format specified
>>     #     by --compressed-debug-sections-format (and if that is "none", well,
>>     #     you still get what you asked for).
>>     #
>>     #  - "sizewin" - compress if there's a size win.  Like "yes", if the format is
>>     #    "none", well, this becomes a nop.
>>
>> ... then the semantics of mixing these options, or what happens when you repeat them
>> would be much more obvious:
>>
>>    - You can specify '--compressed-debug-sections-format' multiple times.  The last wins.
>>
>>    - You can specify '--compress-debug-sections' multiple times.  The last wins.
>>
>>    - Changing '--compressed-debug-sections-format' does not affect the value of '--compress-debug-sections'.
>>
>>    - Changing '--compress-debug-sections' does not affect the value of '--compressed-debug-sections-format'
>>
>>
>> Now, we can't use those option names with that meaning, though, because
>> '--compress-debug-sections' today already has the meaning of selecting
>> the compression format.  But that just means we would need to pick different
>> names, like for example:
>>
>>   --compress-debug-sections=zlib|zstd|none
>>
>>     # Iff we're compressing, what format shall we use?
>>
>>   --nocompress-debug-sections
>>
>>     # Shorthand for --compress-debug-sections=none
>>
>>   --compress-debug-sections-when=never|always|sizewin
>>
>>     # Are we compressing debug sections?  When?
>>     #
>>     #  - "never" - never, we're not compressing.
>>     #
>>     #  - "always" - always compress, using the format specified by
>>     #     by --compress-debug-sections (and if that is "none", well,
>>     #     you still get what you asked for).
>>     #
>>     #  - "sizewin", compress if there's a size win.  Like "always", respects
>>     #    the format specified by --compress-debug-sections.
>>
>>
>> The semantics of mixing these options, or what happens when you repeat them
>> would be obvious in the same way in the other options naming earlier:
>>
>>    - You can specify '--compress-debug-sections' multiple times.  The last wins.
>>
>>    - You can specify '--compress-debug-sections-when' multiple times.  The last wins.
>>
>>    - Changing '--compress-debug-sections' does not affect the value of '--compress-debug-sections-when'.
>>
>>    - Changing '--compress-debug-sections-when' does not affect the value of '--compressed-debug-sections'
>>
>> You end up with two different ways to disable compressing debug sections,
>> but that seems OK to me.
> 
> That would be okay with me as well, but (in part to avoid ...
> 
>> All that would be left would be bikeshed on the new option name.
> 
> ... this aspect) I fail to see why a single option won't do:
> 
> --compress-debug-sections=zlib|zstd|no|yes|sizewin
> 
> Since the sub-options are distinct sets (alongside "no" one could add "none"
> if wanted, to separate "method: no compression" from "don't compress", which
> I think would then also eliminate the ambiguity referred to earlier on
> [still visible in context above]), within each set your "last wins" would
> apply equally. And in fact that was the main reason for my original response
> back to Tom: I view the proliferation of command line options (not just
> here, but in general) as a scalability issue. Therefore my rule of thumb is
> that things which belong together are best grouped under a single top-level
> option.

OK, so say we have --compress-debug-sections=none|zlib|zstd|no|yes|sizewin.

We have the two existing aliases, --compress-debug-sections and 
--nocompress-debug-sections, how do we translate those?

Currently --nocompress-debug-sections maps to 
--compress-debug-sections=none, but I suppose that would have to become 
--compress-debug-sections=no, otherwise things will get extremely confusing.

Then --compress-debug-sections currently maps to 
--compress-debug-sections=zlib.  So, should that become 
--compress-debug-sections=zlib,sizewin?  Or just stay 
--compress-debug-sections=zlib?  I'm not sure if we have made things 
clearer or more orthogonal this way.

I'm starting to wonder if not a simple 
--compress-debug-sections=heuristic-sizewin|heuristic-always is the 
solution, in the sense that these two would only influence the 
heuristics and nothing else, which would work well with the existing 
options.

Thanks,
- Tom