Re: [EXTERNAL] Re: [PATCH] AutoFDO: don't set param_early_inliner_max_iterations to 10.

[Richard Biener via Gcc-patches]

> Am 31.01.2022 um 22:05 schrieb Eugene Rozenfeld 
> <eugene.rozenf...@microsoft.com>:
> 
> Thank you for the review Richard. Is it ok to commit this change to trunk 
> now or should I wait till GCC 13 development starts?
> This seems like a very low risk change.

Yes, it’s Ok now.

Richard 


> Thanks,
> 
> Eugene
> 
> -----Original Message-----
> From: Richard Biener <richard.guent...@gmail.com> 
> Sent: Monday, January 31, 2022 1:23 AM
> To: Eugene Rozenfeld <eugene.rozenf...@microsoft.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: [EXTERNAL] Re: [PATCH] AutoFDO: don't set 
> param_early_inliner_max_iterations to 10.
> 
>> On Sat, Jan 29, 2022 at 12:24 AM Eugene Rozenfeld via Gcc-patches 
>> <gcc-patches@gcc.gnu.org> wrote:
>> 
>> param_early_inliner_max_iterations specifies the maximum number of 
>> nested indirect inlining iterations performed by early inliner.
>> Normally, the default value is 1.
>> 
>> For AutoFDO this parameter was also used as the number of iteration 
>> for its indirect call promotion loop and the default value was set to 10.
>> While it makes sense to have 10 in the indirect call promotion loop 
>> (we want to make the IR match the profiled binary before actual 
>> annotation) there is no reason to have a special default value for the 
>> regular early inliner.
>> 
>> This change removes the special AutoFDO default value setting for 
>> param_early_inliner_max_iterations while keeping 10 as the number of 
>> iterations for the AutoFDO indirect call promotion loop.
>> 
>> This change improves a simple fibonacci benchmark in AutoFDO mode by 
>> 15% on x86_64-pc-linux-gnu.
>> 
>> Tested on x86_64-pc-linux-gnu.
> 
> OK.
> 
> Richard.
> 
>> gcc/ChangeLog:
>>        * auto-profile.cc (auto_profile): Hard-code the number of iterations 
>> (10).
>> 
>> gcc/ChangeLog:
>>        * opt.cc (common_handle_option): Don't set 
>> param_early_inliner_max_iterations to 10 for AutoFDO.
>> ---
>> gcc/auto-profile.cc | 3 +--
>> gcc/opts.cc         | 2 --
>> 2 files changed, 1 insertion(+), 4 deletions(-)
>> 
>> diff --git a/gcc/auto-profile.cc b/gcc/auto-profile.cc index 
>> 0bfaae7b091..c7cee639c85 100644
>> --- a/gcc/auto-profile.cc
>> +++ b/gcc/auto-profile.cc
>> @@ -1644,8 +1644,7 @@ auto_profile (void)
>>        function before annotation, so the profile inside bar@loc_foo2
>>        will be useful.  */
>>     autofdo::stmt_set promoted_stmts;
>> -    for (int i = 0; i < opt_for_fn (node->decl,
>> -                                   param_early_inliner_max_iterations); i++)
>> +    for (int i = 0; i < 10; i++)
>>       {
>>         if (!flag_value_profile_transformations
>>             || !autofdo::afdo_vpt_for_early_inline (&promoted_stmts)) 
>> diff --git a/gcc/opts.cc b/gcc/opts.cc index 17e1884f0e4..f6f6a8e1709 
>> 100644
>> --- a/gcc/opts.cc
>> +++ b/gcc/opts.cc
>> @@ -2899,8 +2899,6 @@ common_handle_option (struct gcc_options *opts,
>>     case OPT_fauto_profile:
>>       enable_fdo_optimizations (opts, opts_set, value);
>>       SET_OPTION_IF_UNSET (opts, opts_set, flag_profile_correction, value);
>> -      SET_OPTION_IF_UNSET (opts, opts_set,
>> -                          param_early_inliner_max_iterations, 10);
>>       break;
>> 
>>     case OPT_fprofile_generate_:
>> --
>> 2.25.1