Richard Biener <rguent...@suse.de> writes: > On Mon, 4 Nov 2019, Segher Boessenkool wrote: > >> Hi! >> >> On Mon, Nov 04, 2019 at 02:31:43PM +0800, Jiufu Guo wrote: >> > 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. >> >> > * gcc/config/rs6000/rs6000.opt (unroll_small_loops): New internal flag. >> >> That's the declaration of a variable. A command line flag is something >> like -munroll-small-loops. Do we want a command line option like that? >> It makes testing simpler. >> >> > - /* 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. */ >> > 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; >> >> That includes -Os? > > It also re-introduces the exactly same issue as the --param with LTO. Thanks Richard, This flag (unroll_small_loops) is saved/restored cl_target_option it can distigush different CU. I had a test, it works when -flto for multi-sources.
Jiufu BR. > >> I think you shouldn't always set it to some value, only enable it where >> you want to enable it. If you make a command line option for it this is >> especially simple (the table in common/config/rs6000/rs6000-common.c). >> >> > +static unsigned >> > +rs6000_loop_unroll_adjust (unsigned nunroll, struct loop * loop) >> > +{ >> > + if (unroll_small_loops) >> > + { >> > + /* TODO: This is hardcoded to 10 right now. It can be refined, for >> > + example we may want to unroll very small loops more times (4 perhaps). >> > + We also should use a PARAM for this. */ >> > + if (loop->ninsns <= 10) >> > + return MIN (2, nunroll); >> > + else >> > + return 0; >> > + } >> >> (Add an empty line here?) >> >> > + return nunroll; >> > +} >> >> Great :-) >> >> > @@ -23472,6 +23488,7 @@ rs6000_function_specific_save (struct >> > cl_target_option *ptr, >> > { >> > ptr->x_rs6000_isa_flags = opts->x_rs6000_isa_flags; >> > ptr->x_rs6000_isa_flags_explicit = opts->x_rs6000_isa_flags_explicit; >> > + ptr->x_unroll_small_loops = opts->x_unroll_small_loops; >> > } >> >> Yeah we shouldn't need to add that, this should all be automatic. >> >> >> Segher >>