From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) by sourceware.org (Postfix) with ESMTP id 0535F3858D35 for ; Wed, 4 Jan 2023 14:03:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0535F3858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=kernel.crashing.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=kernel.crashing.org Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.14.1) with ESMTP id 304E2RkJ028796; Wed, 4 Jan 2023 08:02:27 -0600 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 304E2Q2d028793; Wed, 4 Jan 2023 08:02:26 -0600 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Wed, 4 Jan 2023 08:02:26 -0600 From: Segher Boessenkool To: "Kewen.Lin" Cc: GCC Patches , David Edelsohn , Peter Bergner Subject: Re: [PATCH] rs6000: Don't use optimize_function_for_speed_p too early [PR108184] Message-ID: <20230104140226.GJ25951@gate.crashing.org> References: <197abd1f-081c-3206-4dd5-45f0b098612a@linux.ibm.com> <20230104104648.GI25951@gate.crashing.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi! On Wed, Jan 04, 2023 at 08:15:03PM +0800, Kewen.Lin wrote: > on 2023/1/4 18:46, Segher Boessenkool wrote: > >> @@ -25604,7 +25602,9 @@ rs6000_call_aix (rtx value, rtx func_desc, rtx tlsarg, rtx cookie) > >> > >> /* Can we optimize saving the TOC in the prologue or > >> do we need to do it at every call? */ > >> - if (TARGET_SAVE_TOC_INDIRECT && !cfun->calls_alloca) > >> + if (TARGET_SAVE_TOC_INDIRECT > >> + && !cfun->calls_alloca > >> + && optimize_function_for_speed_p (cfun)) > >> cfun->machine->save_toc_in_prologue = true; > > > > Is this correct? If so, it really needs a separate testcase. > > Yes, it just moves the condition from: > > --- a/gcc/config/rs6000/rs6000.cc > +++ b/gcc/config/rs6000/rs6000.cc > @@ -3978,8 +3978,7 @@ rs6000_option_override_internal (bool global_init_p) > /* If we can shrink-wrap the TOC register save separately, then use > -msave-toc-indirect unless explicitly disabled. */ > if ((rs6000_isa_flags_explicit & OPTION_MASK_SAVE_TOC_INDIRECT) == 0 > - && flag_shrink_wrap_separate > - && optimize_function_for_speed_p (cfun)) > + && flag_shrink_wrap_separate) > rs6000_isa_flags |= OPTION_MASK_SAVE_TOC_INDIRECT; > > here. That "just" reinforces that this really needs a testcase! It is all action at a distance, none of this is trivial (if it was there would not be a bug here in the first place, of course). > I tried to find one test case before, but failed to find one which is not fragile > to test. And I thought the associated test case has demonstrated why the use of > optimize_function_for_{speed,size}_p is too early in function > rs6000_option_override_internal, so I gave up then. Do you worry about that we > could revert it unexpectedly in future and no sensitive test case is on it? I worry that it might contradict what some other code does. I also worry that it just is not a sensible thing to do. I do not worry that your patch is not an improvement. But the resulting code more clearly (than the original) is problematic. Where is r2 saved to the frame if save_toc_in_prologue is false? Segher