Hi Jeff,
Thanks for the patch, I learned a lot from it. Some nits embedded.
on 2019/11/4 下午2:31, Jiufu Guo wrote:
> Hi,
>
> In this patch, loop unroll adjust hook is introduced for powerpc. We can do
> target related hueristic adjustment in this hook. In this patch, small loops
> is unrolled 2 times for O2 and O3 by default. With this patch, we can see
> some improvement for spec2017. This patch enhanced a little for [Patch V2] to
> enable small loops unroll for O3 by default like O2.
>
> Bootstrapped and regtested on powerpc64le. Is this ok for trunk?
>
> Jiufu
> BR.
>
> gcc/
> 2019-11-04 Jiufu Guo <guoji...@linux.ibm.com>
>
> PR tree-optimization/88760
> * config/rs6000/rs6000.c (rs6000_option_override_internal): Remove
> code which changes PARAM_MAX_UNROLL_TIMES and PARAM_MAX_UNROLLED_INSNS.
> (TARGET_LOOP_UNROLL_ADJUST): Add loop unroll adjust hook.
> (rs6000_loop_unroll_adjust): New hook for loop unroll adjust.
> Unrolling small loop 2 times for -O2 and -O3.
> (rs6000_function_specific_save): Save unroll_small_loops flag.
> (rs6000_function_specific_restore): Restore unroll_small_loops flag.
> * gcc/config/rs6000/rs6000.opt (unroll_small_loops): New internal flag.
>
>
> gcc.testsuite/
> 2019-11-04 Jiufu Guo <guoji...@linux.ibm.com>
>
> PR tree-optimization/88760
> * gcc.dg/pr59643.c: Update back to r277550.
>
> ---
> gcc/config/rs6000/rs6000.c | 38 ++++++++++++++++++++++++++++----------
> gcc/config/rs6000/rs6000.opt | 7 +++++++
> gcc/testsuite/gcc.dg/pr59643.c | 3 ---
> 3 files changed, 35 insertions(+), 13 deletions(-)
>
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index 9ed5151..5e1a75d 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -1428,6 +1428,9 @@ static const struct attribute_spec
> rs6000_attribute_table[] =
> #undef TARGET_VECTORIZE_DESTROY_COST_DATA
> #define TARGET_VECTORIZE_DESTROY_COST_DATA rs6000_destroy_cost_data
>
> +#undef TARGET_LOOP_UNROLL_ADJUST
> +#define TARGET_LOOP_UNROLL_ADJUST rs6000_loop_unroll_adjust
> +
> #undef TARGET_INIT_BUILTINS
> #define TARGET_INIT_BUILTINS rs6000_init_builtins
> #undef TARGET_BUILTIN_DECL
> @@ -4540,25 +4543,20 @@ rs6000_option_override_internal (bool global_init_p)
> global_options.x_param_values,
> global_options_set.x_param_values);
>
> - /* unroll very small loops 2 time if no -funroll-loops. */
> + /* If funroll-loops is not enabled explicitly, then enable small loops
> + unrolling for -O2, and do not turn fweb or frename-registers on. */
"for -O2" -> "for -O2 and up"? since I noticed it checks "optimize >=2" later.
> if (!global_options_set.x_flag_unroll_loops
> && !global_options_set.x_flag_unroll_all_loops)
> {
> - maybe_set_param_value (PARAM_MAX_UNROLL_TIMES, 2,
> - global_options.x_param_values,
> - global_options_set.x_param_values);
> -
> - maybe_set_param_value (PARAM_MAX_UNROLLED_INSNS, 20,
> - global_options.x_param_values,
> - global_options_set.x_param_values);
> + unroll_small_loops = optimize >= 2 ? 1 : 0;
Maybe simpler with "unroll_small_loops = flag_unroll_loops"?
>
> - /* If fweb or frename-registers are not specificed in command-line,
> - do not turn them on implicitly. */
> if (!global_options_set.x_flag_web)
> global_options.x_flag_web = 0;
> if (!global_options_set.x_flag_rename_registers)
> global_options.x_flag_rename_registers = 0;
> }
> + else
> + unroll_small_loops = 0;
Could we initialize this in rs6000.opt as zero?
BR,
Kewen
