Re: [PATCH] rs6000: Don't use optimize_function_for_speed_p too early [PR108184]

["Kewen.Lin" <linkw@linux.ibm.com>]

Hi Segher,

Thanks for the comments.

on 2023/1/4 18:46, Segher Boessenkool wrote:
> On Wed, Jan 04, 2023 at 05:20:14PM +0800, Kewen.Lin wrote:
>> As Honza pointed out in [1], the current uses of function
>> optimize_function_for_speed_p in rs6000_option_override_internal
>> are too early, since the query results from the functions
>> optimize_function_for_{speed,size}_p could be changed later due
>> to profile feedback and some function attributes handlings etc.
>>
>> This patch is to move optimize_function_for_speed_p to all the
>> use places of the corresponding flags, which follows the existing
>> practices.  Maybe we can cache it somewhere at an appropriate
>> timing, but that's another thing.
> 
>> @@ -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.

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?


BR,
Kewen