On Thu, May 28, 2020 at 10:52 AM guojiufu <guoji...@linux.ibm.com> wrote:
>
> From: Jiufu Guo <guoji...@linux.ibm.com>
>
> Currently GIMPLE complete unroller(cunroll) is checking
> flag_unroll_loops and flag_peel_loops to see if allow size growth.
> Beside affects curnoll, flag_unroll_loops also controls RTL unroler.
> To have more freedom to control cunroll and RTL unroller, this patch
> introduces flag_cunroll_grow_size. With this patch, we can control
> cunroll and RTL unroller indepently.
>
> Bootstrap/regtest pass on powerpc64le. OK for trunk? And backport to
> GCC10 after week?
>
> gcc/ChangeLog
> 2020-02-28 Jiufu Guo <guoji...@linux.ibm.com>
>
> * common.opt (flag_cunroll_grow_size): New flag.
> * toplev.c (process_options): Set flag_cunroll_grow_size.
> * tree-ssa-loop-ivcanon.c (pass_complete_unroll::execute):
> Use flag_cunroll_grow_size.
> ---
> gcc/common.opt | 4 ++++
> gcc/toplev.c | 4 ++++
> gcc/tree-ssa-loop-ivcanon.c | 3 +--
> 3 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 4464049fc1f..1d0fa7b1749 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -2856,6 +2856,10 @@ funroll-all-loops
> Common Report Var(flag_unroll_all_loops) Optimization
> Perform loop unrolling for all loops.
>
> +funroll-completely-grow-size
> +Var(flag_cunroll_grow_size) Init(2)
> +; Control cunroll to allow size growth during complete unrolling
> +
So this really adds a new compiler option which would need documenting.
I fear we'll get into bikeshed territory here as well... I originally thought
we can use
Variable
int flag_cunroll_grow_size;
but now realize that does not work well with LTO without adjusting
the awk scripts to generate option saving/restoring. For your patch
you'd need to add 'Optimization' to get the flag streamed properly,
you should also verify the target adjustment done in the backend
is reflected in LTO mode.
> ; Nonzero means that loop optimizer may assume that the induction variables
> ; that control loops do not overflow and that the loops with nontrivial
> ; exit condition are not infinite
> diff --git a/gcc/toplev.c b/gcc/toplev.c
> index 96316fbd23b..8d52358efdd 100644
> --- a/gcc/toplev.c
> +++ b/gcc/toplev.c
> @@ -1474,6 +1474,10 @@ process_options (void)
> if (flag_unroll_all_loops)
> flag_unroll_loops = 1;
>
> + /* Allow cunroll to grow size accordingly. */
> + if (flag_cunroll_grow_size == AUTODETECT_VALUE)
> + flag_cunroll_grow_size = flag_unroll_loops || flag_peel_loops;
> +
Any reason to not use EnabledBy(funroll-loops || fpeel-loops)?
> /* web and rename-registers help when run after loop unrolling. */
> if (flag_web == AUTODETECT_VALUE)
> flag_web = flag_unroll_loops;
> diff --git a/gcc/tree-ssa-loop-ivcanon.c b/gcc/tree-ssa-loop-ivcanon.c
> index 8ab6ab3330c..d6a4617a6a1 100644
> --- a/gcc/tree-ssa-loop-ivcanon.c
> +++ b/gcc/tree-ssa-loop-ivcanon.c
> @@ -1603,8 +1603,7 @@ pass_complete_unroll::execute (function *fun)
> re-peeling the same loop multiple times. */
> if (flag_peel_loops)
> peeled_loops = BITMAP_ALLOC (NULL);
> - unsigned int val = tree_unroll_loops_completely (flag_unroll_loops
> - || flag_peel_loops
> + unsigned int val = tree_unroll_loops_completely (flag_cunroll_grow_size
> || optimize >= 3, true);
Given we check optimize >= 3 here please enable the flag by default
at O3+ via opts.c:default_options_table and also elide the optimize >= 3
check. That way -fno-unroll-completely-grow-size would have the desired effect.
Now back to the option name ... if we expose the option we should apply
some forward looking. Currently cunroll cannot be disabled or enabled
with a flag and the desired new flag simply tunes one knob on it. How
about adding
-fcomplete-unroll-loops[=may-grow]
to be able to further extend this later (there's the knob to only unroll
non-outermost loops and the knob whether to unroll loops where
intermediate exits are not statically predicted - incompletely controlled
by -fpeel-loops). There's unfortunately no existing examples that allows
multiple flags like -fcomlete-unroll-loops=may-grow,outer other than
the sanitizers which have manual option parsing.
So if there's no good suggestion from option folks maybe go with
-fcomplete-unroll-loops-may-grow
(ick). And on a second thought -fcomplete-unroll-loops[=...] should
be -funroll-loops[={complete,may-grow,all}] to cover all unrolling
bases?
I really hate to explode the number of options users have to
consider optimizing their code ...
So if we can defer all this thinking and make a non-option flag
variable work that would be best IMHO.
Richard.
> if (peeled_loops)
> {
> --
> 2.17.1
>
