Re: [PATCH] Fix PR92046

[Christophe Lyon <christophe dot lyon at linaro dot org>]

On Thu, 10 Oct 2019 at 16:01, Richard Biener <rguenther@suse.de> wrote:

>
> The following fixes a few param adjustments that are made based on
> per-function adjustable flags by moving the adjustments to their
> users.  Semantics change in some minor ways but that's allowed
> for --params.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.
>
> Hi,

This generates several regressions.
On aarch64:
FAIL:  gcc.target/aarch64/vect_fp16_1.c scan-assembler-times
fadd\tv[0-9]+.8h 2

on arm-linux-gnueabihf:
FAIL: gcc.dg/vect/vect-align-1.c -flto -ffat-lto-objects
 scan-tree-dump-times vect "vectorized 1 loops" 1
FAIL: gcc.dg/vect/vect-align-1.c scan-tree-dump-times vect "vectorized 1
loops" 1
FAIL: gcc.dg/vect/vect-align-2.c -flto -ffat-lto-objects
 scan-tree-dump-times vect "vectorized 1 loops" 1
FAIL: gcc.dg/vect/vect-align-2.c scan-tree-dump-times vect "vectorized 1
loops" 1

on armeb-linux-gnueabihf, many (316) like:
FAIL: gcc.dg/vect/O3-vect-pr34223.c scan-tree-dump-times vect "vectorized 1
loops" 1
FAIL: gcc.dg/vect/fast-math-pr35982.c scan-tree-dump-times vect "vectorized
1 loops" 1

still on armeb-linux-gnueabihf:
    g++.dg/vect/pr33426-ivdep-2.cc  -std=c++14  (test for warnings, line )
    g++.dg/vect/pr33426-ivdep-2.cc  -std=c++17  (test for warnings, line )
    g++.dg/vect/pr33426-ivdep-2.cc  -std=c++2a  (test for warnings, line )
    g++.dg/vect/pr33426-ivdep-2.cc  -std=c++98  (test for warnings, line )
    g++.dg/vect/pr33426-ivdep-3.cc    (test for warnings, line )
    g++.dg/vect/pr33426-ivdep-4.cc    (test for warnings, line )
    g++.dg/vect/pr33426-ivdep.cc  -std=c++14  (test for warnings, line )
    g++.dg/vect/pr33426-ivdep.cc  -std=c++17  (test for warnings, line )
    g++.dg/vect/pr33426-ivdep.cc  -std=c++2a  (test for warnings, line )
    g++.dg/vect/pr33426-ivdep.cc  -std=c++98  (test for warnings, line )

    gfortran.dg/vect/no-vfa-pr32377.f90   -O   scan-tree-dump-times vect
"vectorized 2 loops" 1
    gfortran.dg/vect/pr19049.f90   -O   scan-tree-dump-times vect
"vectorized 1 loops" 1
    gfortran.dg/vect/pr32377.f90   -O   scan-tree-dump-times vect
"vectorized 2 loops" 1
    gfortran.dg/vect/vect-2.f90   -O   scan-tree-dump-times vect
"vectorized 3 loops" 1
    gfortran.dg/vect/vect-3.f90   -O   scan-tree-dump-times vect "Alignment
of access forced using versioning" 3
    gfortran.dg/vect/vect-4.f90   -O   scan-tree-dump-times vect "accesses
have the same alignment." 1
    gfortran.dg/vect/vect-4.f90   -O   scan-tree-dump-times vect
"vectorized 1 loops" 1
    gfortran.dg/vect/vect-5.f90   -O   scan-tree-dump-times vect "Alignment
of access forced using versioning." 2
    gfortran.dg/vect/vect-5.f90   -O   scan-tree-dump-times vect
"vectorized 1 loops" 1

Christophe


Richard.
>
> 2019-10-10  Richard Biener  <rguenther@suse.de>
>
>         PR middle-end/92046
>         * opts.c (finish_options): Do not influence global --params
>         from options that are adjustable per function.
>         * tree-vect-data-refs.c (vect_enhance_data_refs_alignment):
>         Apply --param adjustment based on active cost-model.
>         * tree-ssa-phiopt.c (cond_if_else_store_replacement): Disable
>         further store-sinking when vectorization or if-conversion
>         are not enabled.
>
> Index: gcc/opts.c
> ===================================================================
> --- gcc/opts.c  (revision 276795)+++ gcc/opts.c (working copy)
> +++ gcc/opts.c  (working copy)
> @@ -1123,24 +1123,6 @@ finish_options (struct gcc_options *opts
>        && !opts_set->x_flag_reorder_functions)
>      opts->x_flag_reorder_functions = 1;
>
> -  /* Tune vectorization related parametees according to cost model.  */
> -  if (opts->x_flag_vect_cost_model == VECT_COST_MODEL_CHEAP)
> -    {
> -      maybe_set_param_value (PARAM_VECT_MAX_VERSION_FOR_ALIAS_CHECKS,
> -            6, opts->x_param_values, opts_set->x_param_values);
> -      maybe_set_param_value (PARAM_VECT_MAX_VERSION_FOR_ALIGNMENT_CHECKS,
> -            0, opts->x_param_values, opts_set->x_param_values);
> -      maybe_set_param_value (PARAM_VECT_MAX_PEELING_FOR_ALIGNMENT,
> -            0, opts->x_param_values, opts_set->x_param_values);
> -    }
> -
> -  /* Set PARAM_MAX_STORES_TO_SINK to 0 if either vectorization or
> if-conversion
> -     is disabled.  */
> -  if ((!opts->x_flag_tree_loop_vectorize &&
> !opts->x_flag_tree_slp_vectorize)
> -       || !opts->x_flag_tree_loop_if_convert)
> -    maybe_set_param_value (PARAM_MAX_STORES_TO_SINK, 0,
> -                           opts->x_param_values,
> opts_set->x_param_values);
> -
>    /* The -gsplit-dwarf option requires -ggnu-pubnames.  */
>    if (opts->x_dwarf_split_debug_info)
>      opts->x_debug_generate_pub_sections = 2;
> Index: gcc/tree-vect-data-refs.c
> ===================================================================
> --- gcc/tree-vect-data-refs.c   (revision 276795)
> +++ gcc/tree-vect-data-refs.c   (working copy)
> @@ -2075,6 +2075,8 @@ vect_enhance_data_refs_alignment (loop_v
>          {
>            unsigned max_allowed_peel
>              = PARAM_VALUE (PARAM_VECT_MAX_PEELING_FOR_ALIGNMENT);
> +         if (flag_vect_cost_model == VECT_COST_MODEL_CHEAP)
> +           max_allowed_peel = 0;
>            if (max_allowed_peel != (unsigned)-1)
>              {
>                unsigned max_peel = npeel;
> @@ -2168,15 +2170,16 @@ vect_enhance_data_refs_alignment (loop_v
>    /* (2) Versioning to force alignment.  */
>
>    /* Try versioning if:
> -     1) optimize loop for speed
> +     1) optimize loop for speed and the cost-model is not cheap
>       2) there is at least one unsupported misaligned data ref with an
> unknown
>          misalignment, and
>       3) all misaligned data refs with a known misalignment are supported,
> and
>       4) the number of runtime alignment checks is within reason.  */
>
> -  do_versioning =
> -       optimize_loop_nest_for_speed_p (loop)
> -       && (!loop->inner); /* FORNOW */
> +  do_versioning
> +    = (optimize_loop_nest_for_speed_p (loop)
> +       && !loop->inner /* FORNOW */
> +       && flag_vect_cost_model > VECT_COST_MODEL_CHEAP);
>
>    if (do_versioning)
>      {
> @@ -3641,13 +3644,15 @@ vect_prune_runtime_alias_test_list (loop
>      dump_printf_loc (MSG_NOTE, vect_location,
>                      "improved number of alias checks from %d to %d\n",
>                      may_alias_ddrs.length (), count);
> -  if ((int) count > PARAM_VALUE (PARAM_VECT_MAX_VERSION_FOR_ALIAS_CHECKS))
> +  unsigned limit = PARAM_VALUE (PARAM_VECT_MAX_VERSION_FOR_ALIAS_CHECKS);
> +  if (flag_simd_cost_model == VECT_COST_MODEL_CHEAP)
> +    limit = default_param_value
> +             (PARAM_VECT_MAX_VERSION_FOR_ALIAS_CHECKS) * 6 / 10;
> +  if (count > limit)
>      return opt_result::failure_at
>        (vect_location,
> -       "number of versioning for alias "
> -       "run-time tests exceeds %d "
> -       "(--param vect-max-version-for-alias-checks)\n",
> -       PARAM_VALUE (PARAM_VECT_MAX_VERSION_FOR_ALIAS_CHECKS));
> +       "number of versioning for alias run-time tests exceeds %d "
> +       "(--param vect-max-version-for-alias-checks)\n", limit);
>
>    return opt_result::success ();
>  }
> Index: gcc/tree-ssa-phiopt.c
> ===================================================================
> --- gcc/tree-ssa-phiopt.c       (revision 276795)
> +++ gcc/tree-ssa-phiopt.c       (working copy)
> @@ -2467,7 +2467,11 @@ cond_if_else_store_replacement (basic_bl
>                                                  then_assign, else_assign);
>      }
>
> -  if (MAX_STORES_TO_SINK == 0)
> +  /* If either vectorization or if-conversion is disabled then do
> +     not sink any stores.  */
> +  if (MAX_STORES_TO_SINK == 0
> +      || (!flag_tree_loop_vectorize && !flag_tree_slp_vectorize)
> +      || !flag_tree_loop_if_convert)
>      return false;
>
>    /* Find data references.  */
>