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 03FB03858430 for ; Tue, 12 Oct 2021 14:03:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 03FB03858430 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.cz 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 E9DAC200AD; Tue, 12 Oct 2021 14:03:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1634047421; 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=LGZeg5UybZK3HprO9ujaVUCG1WQpi7Crgvmm83VVRFI=; b=N8b+KEprSWzOm6ZMpoOBFX/6pkV/7MK96SGcwF74qOpN2eCQcuiZ2EDNngjgNTHNSB4H7V iZu+ByyF8hJ8yzGafio95LZ8MbuqtvM8k2pn9aolKvdKo9McwjXDHl+aaDEgBJY5izdRJY XFQQg90jwzelIPalPK+IpU1x90vYY58= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1634047421; 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=LGZeg5UybZK3HprO9ujaVUCG1WQpi7Crgvmm83VVRFI=; b=ZNF1PNdt02EB94wl5BQuHYku+0JFeSZPMolEAAwMmFXAR+CGWmGpY36Nlm9iSdERgcSu5w llAp3DfIyXXf/LDA== 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 CD6DB13BEC; Tue, 12 Oct 2021 14:03:41 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id HpEGMb2VZWGOegAAMHmgww (envelope-from ); Tue, 12 Oct 2021 14:03:41 +0000 Message-ID: <62c7a52b-0b17-9ebd-2d88-b161737ba84f@suse.cz> Date: Tue, 12 Oct 2021 16:03:41 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.2 Subject: Re: [PATCH] Fix handling of flag_rename_registers. Content-Language: en-US To: Richard Biener , "Joseph S. Myers" Cc: GCC Patches References: <12fd0c18-79e0-d20d-2973-92639071c050@suse.cz> From: =?UTF-8?Q?Martin_Li=c5=a1ka?= In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Oct 2021 14:03:45 -0000 On 10/12/21 15:37, Richard Biener wrote: > On Tue, Oct 12, 2021 at 2:18 PM Martin Liška wrote: >> >> Hello. >> >> The option is disabled in rs6000 target with: >> >> { OPT_LEVELS_ALL, OPT_frename_registers, NULL, 0 }, >> >> Thus, we have to do an auto-detection only if it's really unset and also >> equal to the Init value. >> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. >> And the problematic test-case works on ppc64le. >> >> Ready to be installed? > > Hmm, I can see how it fixes the reported problem but I think the > thing is fragile. You are fully right, it's quite fragile. > I wonder if we can express things like > > + if (!OPTION_SET_P (flag_web)) > flag_web = flag_unroll_loops; > > or > > + if (!OPTION_SET_P (flag_rename_registers)) > flag_rename_registers = flag_unroll_loops; > > by adding EnabledBy(funroll-loops) to the respective options instead > (and funroll-loops EnabledBy(funroll-all-loops)) Testing that approach, I like it. Note that my fix: if (!OPTION_SET_P (flag_rename_registers) && flag_rename_registers) won't work if one target sets flag_rename_registers = 1 and another to flag_rename_registers = 0. Then one can't use Init setting a proper default value. > > All SET_OPTION_IF_UNSET are fragile with respect to target overrides > (-fprofile-use does a lot of those for example). > > I suppose opts_set could also record whether the target overrided > sth with its option_optimization_table. I can experiment with a patch where SET_OPTION_IF_UNSET modified opts_set. Thanks for clever feedback. Martin > > Richard. > >> Thanks, >> Martin >> >> PR target/102688 >> >> gcc/ChangeLog: >> >> * common.opt: Enable flag_rename_registers by default. >> * toplev.c (process_options): Auto-detect flag_rename_registers >> only if it is not turned off in a target. >> --- >> gcc/common.opt | 2 +- >> gcc/toplev.c | 3 ++- >> 2 files changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/gcc/common.opt b/gcc/common.opt >> index 4099effcc80..2c6be1bdd36 100644 >> --- a/gcc/common.opt >> +++ b/gcc/common.opt >> @@ -2399,7 +2399,7 @@ Common Var(flag_live_range_shrinkage) Init(0) Optimization >> Relief of register pressure through live range shrinkage. >> >> frename-registers >> -Common Var(flag_rename_registers) Optimization >> +Common Var(flag_rename_registers) Init(1) Optimization >> Perform a register renaming optimization pass. >> >> fschedule-fusion >> diff --git a/gcc/toplev.c b/gcc/toplev.c >> index 167feac2583..ee7d8854f90 100644 >> --- a/gcc/toplev.c >> +++ b/gcc/toplev.c >> @@ -1335,7 +1335,8 @@ process_options (bool no_backend) >> if (!OPTION_SET_P (flag_web)) >> flag_web = flag_unroll_loops; >> >> - if (!OPTION_SET_P (flag_rename_registers)) >> + /* The option can be turned off in a target. */ >> + if (!OPTION_SET_P (flag_rename_registers) && flag_rename_registers) >> flag_rename_registers = flag_unroll_loops; >> >> if (flag_non_call_exceptions) >> -- >> 2.33.0 >>