From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id 847E03858D32 for ; Mon, 27 Feb 2023 23:24:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 847E03858D32 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id A04D51F45E; Mon, 27 Feb 2023 23:24:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1677540281; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=YZsFVyWaJ1UtvhvxgeDk98luYyO+II85eommd6Wd4k4=; b=roCZ1VMQZKPXpTWsJamwqRXRewzyWYtZeuNGfaLrpJa0/892i2btuGcBFeOJ6N+OUjgjh3 njrNGVwb5fR87hsZzXEOCQSaIvNntdbf+eFxK60z6kohTI8D9yufJUxNe5di0hpAu8r5PZ YI0t/2Sih9ugrxHSTm9bt6mTINEho90= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1677540281; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=YZsFVyWaJ1UtvhvxgeDk98luYyO+II85eommd6Wd4k4=; b=36A492IQjgQ/V6Cp6zaDC7jkEw92FzntPQBkEUiCxVcT1KMyoIp9D04ufiN4ItxHYdAAEJ TIx70KBXnRno0dBw== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 72F0413A43; Mon, 27 Feb 2023 23:24:41 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id ZBrLGrk7/WNfZAAAMHmgww (envelope-from ); Mon, 27 Feb 2023 23:24:41 +0000 Message-ID: <6ad0c7f3-8971-d5b6-682c-8aaefc484db9@suse.de> Date: Tue, 28 Feb 2023 00:24:48 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Subject: Re: [PATCH] gas: Add --force-compress-debug-sections Content-Language: en-US To: Jan Beulich , Pedro Alves Cc: binutils@sourceware.org, Michael Matz , Nick Clifton , Alan Modra References: <20230223124519.4228-1-tdevries@suse.de> <7dcb7bfb-f65d-aed8-78d4-944211ef5127@suse.de> <819f729a-da9c-3b8a-3769-7995c009704b@suse.com> <14a2defc-5371-84bc-2d59-9980755b112a@suse.de> <02dcf47c-4256-c5e5-de9e-814b60da8ce8@suse.com> <7cb226d0-1a91-9bad-181c-46f79c4d6eaf@suse.de> <0c60eef7-c612-ec37-8c3f-36b746ff8d95@suse.de> <711d9da5-8f31-3f1e-530b-5ae01780fcad@suse.de> <70ee7e78-080f-ddce-9916-608bd8733e66@palves.net> From: Tom de Vries In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-6.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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