diff mbox series

Fix PR92046

Message ID nycvar.YFH.7.76.1910101559500.5566@zhemvz.fhfr.qr
State New
Headers show
Series Fix PR92046 | expand

Commit Message

Richard Biener Oct. 10, 2019, 2:01 p.m. UTC 
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.

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.

Comments

Christophe Lyon Oct. 11, 2019, 9:59 a.m. UTC | #1 
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.  */
>
Rainer Orth Oct. 11, 2019, 10:19 a.m. UTC | #2 
Hi Christophe,

> 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

that's PR tree-optimization/92066, also seen on sparc, powerpc64, and
ia64.

	Rainer
Richard Biener Oct. 11, 2019, 10:43 a.m. UTC | #3 
On Fri, 11 Oct 2019, Rainer Orth wrote:

> Hi Christophe,
> 
> > 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
> 
> that's PR tree-optimization/92066, also seen on sparc, powerpc64, and
> ia64.

Hmm, OK.  There's one obvious bug fixed below, other than that I have
to investigate in more detail.

Committed as obvious.

Richard.

2019-10-11  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/92066
	PR tree-optimization/92046
	* tree-vect-data-refs.c (vect_enhance_data_refs_alignment):
	Fix bogus cost model check.

Index: gcc/tree-vect-data-refs.c
===================================================================
--- gcc/tree-vect-data-refs.c	(revision 276858)
+++ gcc/tree-vect-data-refs.c	(working copy)
@@ -2179,7 +2179,7 @@
   do_versioning
     = (optimize_loop_nest_for_speed_p (loop)
        && !loop->inner /* FORNOW */
-       && flag_vect_cost_model > VECT_COST_MODEL_CHEAP);
+       && flag_vect_cost_model != VECT_COST_MODEL_CHEAP);
 
   if (do_versioning)
     {
Christophe Lyon Oct. 14, 2019, 2:53 p.m. UTC | #4 
On Fri, 11 Oct 2019 at 12:43, Richard Biener <rguenther@suse.de> wrote:

> On Fri, 11 Oct 2019, Rainer Orth wrote:
>
> > Hi Christophe,
> >
> > > 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
> >
> > that's PR tree-optimization/92066, also seen on sparc, powerpc64, and
> > ia64.
>
> Hmm, OK.  There's one obvious bug fixed below, other than that I have
> to investigate in more detail.
>
> Committed as obvious.
>
> Hi Richard,

This patch caused another regression on armeb:
FAIL: gcc.dg/vect/vect-multitypes-11.c -flto -ffat-lto-objects
 scan-tree-dump-times vect "vectorized 1 loops" 1
FAIL: gcc.dg/vect/vect-multitypes-11.c scan-tree-dump-times vect
"vectorized 1 loops" 1
FAIL: gcc.dg/vect/vect-multitypes-12.c -flto -ffat-lto-objects
 scan-tree-dump-times vect "vectorized 1 loops" 1
FAIL: gcc.dg/vect/vect-multitypes-12.c scan-tree-dump-times vect
"vectorized 1 loops" 1

Christophe


 Richard.
>
> 2019-10-11  Richard Biener  <rguenther@suse.de>
>
>         PR tree-optimization/92066
>         PR tree-optimization/92046
>         * tree-vect-data-refs.c (vect_enhance_data_refs_alignment):
>         Fix bogus cost model check.
>
> Index: gcc/tree-vect-data-refs.c
> ===================================================================
> --- gcc/tree-vect-data-refs.c   (revision 276858)
> +++ gcc/tree-vect-data-refs.c   (working copy)
> @@ -2179,7 +2179,7 @@
>    do_versioning
>      = (optimize_loop_nest_for_speed_p (loop)
>         && !loop->inner /* FORNOW */
> -       && flag_vect_cost_model > VECT_COST_MODEL_CHEAP);
> +       && flag_vect_cost_model != VECT_COST_MODEL_CHEAP);
>
>    if (do_versioning)
>      {
>
Richard Biener Oct. 14, 2019, 3:50 p.m. UTC | #5 
On October 14, 2019 4:53:02 PM GMT+02:00, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>On Fri, 11 Oct 2019 at 12:43, Richard Biener <rguenther@suse.de> wrote:
>
>> On Fri, 11 Oct 2019, Rainer Orth wrote:
>>
>> > Hi Christophe,
>> >
>> > > 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
>> >
>> > that's PR tree-optimization/92066, also seen on sparc, powerpc64,
>and
>> > ia64.
>>
>> Hmm, OK.  There's one obvious bug fixed below, other than that I have
>> to investigate in more detail.
>>
>> Committed as obvious.
>>
>> Hi Richard,
>
>This patch caused another regression on armeb:
>FAIL: gcc.dg/vect/vect-multitypes-11.c -flto -ffat-lto-objects
> scan-tree-dump-times vect "vectorized 1 loops" 1
>FAIL: gcc.dg/vect/vect-multitypes-11.c scan-tree-dump-times vect
>"vectorized 1 loops" 1
>FAIL: gcc.dg/vect/vect-multitypes-12.c -flto -ffat-lto-objects
> scan-tree-dump-times vect "vectorized 1 loops" 1
>FAIL: gcc.dg/vect/vect-multitypes-12.c scan-tree-dump-times vect
>"vectorized 1 loops" 1

Are the other ones fixed though? 

Richard. 

>Christophe
>
>
> Richard.
>>
>> 2019-10-11  Richard Biener  <rguenther@suse.de>
>>
>>         PR tree-optimization/92066
>>         PR tree-optimization/92046
>>         * tree-vect-data-refs.c (vect_enhance_data_refs_alignment):
>>         Fix bogus cost model check.
>>
>> Index: gcc/tree-vect-data-refs.c
>> ===================================================================
>> --- gcc/tree-vect-data-refs.c   (revision 276858)
>> +++ gcc/tree-vect-data-refs.c   (working copy)
>> @@ -2179,7 +2179,7 @@
>>    do_versioning
>>      = (optimize_loop_nest_for_speed_p (loop)
>>         && !loop->inner /* FORNOW */
>> -       && flag_vect_cost_model > VECT_COST_MODEL_CHEAP);
>> +       && flag_vect_cost_model != VECT_COST_MODEL_CHEAP);
>>
>>    if (do_versioning)
>>      {
>>
Christophe Lyon Oct. 15, 2019, 7:51 a.m. UTC | #6 
On Mon, 14 Oct 2019 at 17:51, Richard Biener <rguenther@suse.de> wrote:
>
> On October 14, 2019 4:53:02 PM GMT+02:00, Christophe Lyon <christophe.lyon@linaro.org> wrote:
> >On Fri, 11 Oct 2019 at 12:43, Richard Biener <rguenther@suse.de> wrote:
> >
> >> On Fri, 11 Oct 2019, Rainer Orth wrote:
> >>
> >> > Hi Christophe,
> >> >
> >> > > 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
> >> >
> >> > that's PR tree-optimization/92066, also seen on sparc, powerpc64,
> >and
> >> > ia64.
> >>
> >> Hmm, OK.  There's one obvious bug fixed below, other than that I have
> >> to investigate in more detail.
> >>
> >> Committed as obvious.
> >>
> >> Hi Richard,
> >
> >This patch caused another regression on armeb:
> >FAIL: gcc.dg/vect/vect-multitypes-11.c -flto -ffat-lto-objects
> > scan-tree-dump-times vect "vectorized 1 loops" 1
> >FAIL: gcc.dg/vect/vect-multitypes-11.c scan-tree-dump-times vect
> >"vectorized 1 loops" 1
> >FAIL: gcc.dg/vect/vect-multitypes-12.c -flto -ffat-lto-objects
> > scan-tree-dump-times vect "vectorized 1 loops" 1
> >FAIL: gcc.dg/vect/vect-multitypes-12.c scan-tree-dump-times vect
> >"vectorized 1 loops" 1
>
> Are the other ones fixed though?
>
Yes, thanks.

> Richard.
>
> >Christophe
> >
> >
> > Richard.
> >>
> >> 2019-10-11  Richard Biener  <rguenther@suse.de>
> >>
> >>         PR tree-optimization/92066
> >>         PR tree-optimization/92046
> >>         * tree-vect-data-refs.c (vect_enhance_data_refs_alignment):
> >>         Fix bogus cost model check.
> >>
> >> Index: gcc/tree-vect-data-refs.c
> >> ===================================================================
> >> --- gcc/tree-vect-data-refs.c   (revision 276858)
> >> +++ gcc/tree-vect-data-refs.c   (working copy)
> >> @@ -2179,7 +2179,7 @@
> >>    do_versioning
> >>      = (optimize_loop_nest_for_speed_p (loop)
> >>         && !loop->inner /* FORNOW */
> >> -       && flag_vect_cost_model > VECT_COST_MODEL_CHEAP);
> >> +       && flag_vect_cost_model != VECT_COST_MODEL_CHEAP);
> >>
> >>    if (do_versioning)
> >>      {
> >>
>
diff mbox series

Patch

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.  */