On August 12, 2020 10:20:12 AM GMT+02:00, Jakub Jelinek <ja...@redhat.com>
wrote:
>Hi!
>
>As the testcase in the PR shows (not included in the patch, as
>it seems quite fragile to observe unrolling in the IL), the
>introduction of
>flag_cunroll_grow_size broke optimize attribute related to loop
>unrolling.
>The problem is that the new option flag is set (if not set explicitly)
>only
>in process_options and in rs6000_option_override_internal (and there
>only if
>global_init_p). So, this means that while it is Optimization option,
>it
>will only be set based on the command line
>-funroll-loops/-O3/-fpeel-loops
>or -funroll-all-loops, which means that if command line does include
>any of
>those, it is enabled even for functions that will through optimize
>attribute
>have all of those disabled, and if command line does not include those,
>it will not be enabled for functions that will through optimize
>attribute
>have any of those enabled.
>
>process_options is called just once, so IMHO it should be handling only
>non-Optimization option adjustments (various other options suffer from
>that
>too, but as this is a regression from 10.1 on the 10 branch, changing
>those
>is not appropriate). Similarly, rs6000_option_override_internal is
>called
>only once (with global_init_p) and then for target attribute handling,
>but
>not for optimize attribute handling.
>
>This patch moves the unrolling related handling from process_options
>into
>finish_options which is invoked whenever the options are being
>finalized,
>and the rs6000 specific parts into the override_options_after_change
>hook
>which is called for optimize attribute handling (and unfortunately also
>th cfun changes, but what the hook does is cheap) and I've added a call
>to
>that from rs6000_override_options_internal, so it is also called on
>cmdline
>processing and for target attribute.
>
>Furthermore, it stops using AUTODETECT_VALUE, which can work only once,
>and instead uses the global_options_set.x_... flags.
>
>Bootstrapped/regtested on {x86_64,i686,powerpc64{,le}}-linux, ok for
>trunk
>and after a while 10.3?
OK.
Richard.
>2020-08-12 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/96535
> * toplev.c (process_options): Move flag_unroll_loops and
> flag_cunroll_grow_size handling from here to ...
> * opts.c (finish_options): ... here. For flag_cunroll_grow_size,
> don't check for AUTODETECT_VALUE, but instead check
> opts_set->x_flag_cunroll_grow_size.
> * common.opt (funroll-completely-grow-size): Default to 0.
> * config/rs6000/rs6000.c (TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE):
> Redefine.
> (rs6000_override_options_after_change): New function.
> (rs6000_option_override_internal): Call it. Move there the
> flag_cunroll_grow_size, unroll_only_small_loops and
> flag_rename_registers handling.
>
>--- gcc/toplev.c.jj 2020-08-11 14:20:35.179934850 +0200
>+++ gcc/toplev.c 2020-08-11 14:44:06.861586574 +0200
>@@ -1474,16 +1474,6 @@ process_options (void)
> flag_abi_version = 2;
> }
>
>- /* Unrolling all loops implies that standard loop unrolling must
>also
>- be done. */
>- 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 || optimize >= 3;
>-
> /* web and rename-registers help when run after loop unrolling. */
> if (flag_web == AUTODETECT_VALUE)
> flag_web = flag_unroll_loops;
>--- gcc/opts.c.jj 2020-08-11 14:20:35.169934987 +0200
>+++ gcc/opts.c 2020-08-11 14:43:47.578850847 +0200
>@@ -1142,11 +1142,21 @@ finish_options (struct gcc_options *opts
>
>/* Control IPA optimizations based on different -flive-patching level.
>*/
> if (opts->x_flag_live_patching)
>- {
>- control_options_for_live_patching (opts, opts_set,
>- opts->x_flag_live_patching,
>- loc);
>- }
>+ control_options_for_live_patching (opts, opts_set,
>+ opts->x_flag_live_patching,
>+ loc);
>+
>+ /* Unrolling all loops implies that standard loop unrolling must
>also
>+ be done. */
>+ if (opts->x_flag_unroll_all_loops)
>+ opts->x_flag_unroll_loops = 1;
>+
>+ /* Allow cunroll to grow size accordingly. */
>+ if (!opts_set->x_flag_cunroll_grow_size)
>+ opts->x_flag_cunroll_grow_size
>+ = (opts->x_flag_unroll_loops
>+ || opts->x_flag_peel_loops
>+ || opts->x_optimize >= 3);
> }
>
> #define LEFT_COLUMN 27
>--- gcc/common.opt.jj 2020-08-03 22:54:51.328532939 +0200
>+++ gcc/common.opt 2020-08-11 14:42:14.935120568 +0200
>@@ -2884,7 +2884,7 @@ Common Report Var(flag_unroll_all_loops)
> Perform loop unrolling for all loops.
>
> funroll-completely-grow-size
>-Undocumented Var(flag_cunroll_grow_size) Init(2) Optimization
>+Undocumented Var(flag_cunroll_grow_size) Optimization
>; Internal undocumented flag, allow size growth during complete
>unrolling
>
>; Nonzero means that loop optimizer may assume that the induction
>variables
>--- gcc/config/rs6000/rs6000.c.jj 2020-08-10 10:44:20.658560709 +0200
>+++ gcc/config/rs6000/rs6000.c 2020-08-11 15:17:31.443164022 +0200
>@@ -1493,6 +1493,9 @@ static const struct attribute_spec rs600
> #undef TARGET_PROMOTE_FUNCTION_MODE
> #define TARGET_PROMOTE_FUNCTION_MODE rs6000_promote_function_mode
>
>+#undef TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE
>+#define TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE
>rs6000_override_options_after_change
>+
> #undef TARGET_RETURN_IN_MEMORY
> #define TARGET_RETURN_IN_MEMORY rs6000_return_in_memory
>
>@@ -3420,6 +3423,34 @@ rs6000_md_asm_adjust (vec<rtx> &/*output
> return NULL;
> }
>
>+/* This target function is similar to the hook TARGET_OPTION_OVERRIDE
>+ but is called when the optimize level is changed via an attribute
>or
>+ pragma or when it is reset at the end of the code affected by the
>+ attribute or pragma. It is not called at the beginning of
>compilation
>+ when TARGET_OPTION_OVERRIDE is called so if you want to perform
>these
>+ actions then, you should have TARGET_OPTION_OVERRIDE call
>+ TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE. */
>+
>+static void
>+rs6000_override_options_after_change (void)
>+{
>+ /* Explicit -funroll-loops turns -munroll-only-small-loops off, and
>+ turns -frename-registers on. */
>+ if ((global_options_set.x_flag_unroll_loops && flag_unroll_loops)
>+ || (global_options_set.x_flag_unroll_all_loops
>+ && flag_unroll_all_loops))
>+ {
>+ if (!global_options_set.x_unroll_only_small_loops)
>+ unroll_only_small_loops = 0;
>+ if (!global_options_set.x_flag_rename_registers)
>+ flag_rename_registers = 1;
>+ if (!global_options_set.x_flag_cunroll_grow_size)
>+ flag_cunroll_grow_size = 1;
>+ }
>+ else if (!global_options_set.x_flag_cunroll_grow_size)
>+ flag_cunroll_grow_size = flag_peel_loops || optimize >= 3;
>+}
>+
> /* Override command line options.
>
> Combine build-specific configuration information with options
>@@ -4647,23 +4678,6 @@ rs6000_option_override_internal (bool gl
> param_sched_pressure_algorithm,
> SCHED_PRESSURE_MODEL);
>
>- /* Explicit -funroll-loops turns -munroll-only-small-loops off,
>and
>- turns -frename-registers on. */
>- if ((global_options_set.x_flag_unroll_loops &&
>flag_unroll_loops)
>- || (global_options_set.x_flag_unroll_all_loops
>- && flag_unroll_all_loops))
>- {
>- if (!global_options_set.x_unroll_only_small_loops)
>- unroll_only_small_loops = 0;
>- if (!global_options_set.x_flag_rename_registers)
>- flag_rename_registers = 1;
>- if (!global_options_set.x_flag_cunroll_grow_size)
>- flag_cunroll_grow_size = 1;
>- }
>- else
>- if (!global_options_set.x_flag_cunroll_grow_size)
>- flag_cunroll_grow_size = flag_peel_loops || optimize >= 3;
>-
> /* If using typedef char *va_list, signal that
> __builtin_va_start (&ap, 0) can be optimized to
> ap = __builtin_next_arg (0). */
>@@ -4671,6 +4685,8 @@ rs6000_option_override_internal (bool gl
> targetm.expand_builtin_va_start = NULL;
> }
>
>+ rs6000_override_options_after_change ();
>+
>/* If not explicitly specified via option, decide whether to generate
>indexed
> load/store instructions. A value of -1 indicates that the
> initial value of this variable has not been overwritten. During
>
>
> Jakub
