Re: [gomp4 simd, RFC] Simple fix to override vectorization cost estimation.

[Richard Biener <rguenther at suse dot de>]

On Wed, 20 Nov 2013, Sergey Ostanevich wrote:

> Thanks for comments, hope I got all of em.
> Note: I used a LOOP_VINFO_LOC (loop_vinfo) to print the loop location
> but it appears to be 0, so the output is somewhat lousy. The global
> vect_location points somewhere inside the loop, which is not that better.
> Shall we address this separately?

Use vect_location instead.

Note that c-family/ and fortran/ have their own ChangeLog file
and files there don't have a prefix.

Ok with the change to use vect_location.

Thanks,
Richard.

> Sergos
> 
>         * common.opt: Added new option -fsimd-cost-model.
>         * tree-vectorizer.h (unlimited_cost_model): Interface update
>         to rely on particular loop info.
>         * tree-vect-data-refs.c (vect_peeling_hash_insert): Update to
>         unlimited_cost_model call according to new interface.
>         (vect_peeling_hash_choose_best_peeling): Ditto.
>         (vect_enhance_data_refs_alignment): Ditto.
>         * tree-vect-slp.c: Ditto.
>         * tree-vect-loop.c (vect_estimate_min_profitable_iters): Ditto,
>         plus issue a warning in case cost model overrides users' directive.
>         * c-family/c.opt: add openmp-simd warning.
>         * fortran/lang.opt: Ditto.
>         * doc/invoke.texi: Added new openmp-simd warning.
> 
> 
> 
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index 0026683..a85a8ad 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -592,6 +592,10 @@ Wold-style-definition
>  C ObjC Var(warn_old_style_definition) Warning
>  Warn if an old-style parameter definition is used
> 
> +Wopenmp-simd
> +C C++ Var(openmp_simd) Warning EnabledBy(Wall)
> +Warn about simd directive is overridden by vectorizer cost model
> +
>  Woverlength-strings
>  C ObjC C++ ObjC++ Var(warn_overlength_strings) Warning
> LangEnabledBy(C ObjC C++ ObjC++,Wpedantic)
>  Warn if a string is longer than the maximum portable length specified
> by the standard
> diff --git a/gcc/common.opt b/gcc/common.opt
> index d5971df..2b0e9e6 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -2296,6 +2296,10 @@ fvect-cost-model=
>  Common Joined RejectNegative Enum(vect_cost_model)
> Var(flag_vect_cost_model) Init(VECT_COST_MODEL_DEFAULT)
>  Specifies the cost model for vectorization
> 
> +fsimd-cost-model=
> +Common Joined RejectNegative Enum(vect_cost_model)
> Var(flag_simd_cost_model) Init(VECT_COST_MODEL_UNLIMITED)
> +Specifies the vectorization cost model for code marked with simd directive
> +
>  Enum
>  Name(vect_cost_model) Type(enum vect_cost_model) UnknownError(unknown
> vectorizer cost model %qs)
> 
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index c250385..050bd44 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -256,7 +256,7 @@ Objective-C and Objective-C++ Dialects}.
>  -Wlogical-op -Wlong-long @gol
>  -Wmain -Wmaybe-uninitialized -Wmissing-braces
> -Wmissing-field-initializers @gol
>  -Wmissing-include-dirs @gol
> --Wno-multichar  -Wnonnull  -Wno-overflow @gol
> +-Wno-multichar  -Wnonnull  -Wno-overflow -Wopenmp-simd @gol
>  -Woverlength-strings  -Wpacked  -Wpacked-bitfield-compat  -Wpadded @gol
>  -Wparentheses  -Wpedantic-ms-format -Wno-pedantic-ms-format @gol
>  -Wpointer-arith  -Wno-pointer-to-int-cast @gol
> @@ -3318,6 +3318,7 @@ Options} and @ref{Objective-C and Objective-C++
> Dialect Options}.
>  -Wmaybe-uninitialized @gol
>  -Wmissing-braces @r{(only for C/ObjC)} @gol
>  -Wnonnull  @gol
> +-Wopenmp-simd @gol
>  -Wparentheses  @gol
>  -Wpointer-sign  @gol
>  -Wreorder   @gol
> @@ -4804,6 +4805,10 @@ attribute.
>  @opindex Woverflow
>  Do not warn about compile-time overflow in constant expressions.
> 
> +@item -Wopenmp-simd
> +@opindex Wopenm-simd
> +Warn if vectorizer cost model overrides simd directive from user.
> +
>  @item -Woverride-init @r{(C and Objective-C only)}
>  @opindex Woverride-init
>  @opindex Wno-override-init
> diff --git a/gcc/fortran/lang.opt b/gcc/fortran/lang.opt
> index 5e09cbd..b43c48c 100644
> --- a/gcc/fortran/lang.opt
> +++ b/gcc/fortran/lang.opt
> @@ -257,6 +257,10 @@ Wintrinsics-std
>  Fortran Warning
>  Warn on intrinsics not part of the selected standard
> 
> +Wopenmp-simd
> +Fortran Warning
> +; Documented in C
> +
>  Wreal-q-constant
>  Fortran Warning
>  Warn about real-literal-constants with 'q' exponent-letter
> diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
> index 83d1f45..977db43 100644
> --- a/gcc/tree-vect-data-refs.c
> +++ b/gcc/tree-vect-data-refs.c
> @@ -1090,7 +1090,8 @@ vect_peeling_hash_insert (loop_vec_info
> loop_vinfo, struct data_reference *dr,
>        *new_slot = slot;
>      }
> 
> -  if (!supportable_dr_alignment && unlimited_cost_model ())
> +  if (!supportable_dr_alignment
> +      && unlimited_cost_model (LOOP_VINFO_LOOP (loop_vinfo)))
>      slot->count += VECT_MAX_COST;
>  }
> 
> @@ -1200,7 +1201,7 @@ vect_peeling_hash_choose_best_peeling
> (loop_vec_info loop_vinfo,
>     res.peel_info.dr = NULL;
>     res.body_cost_vec = stmt_vector_for_cost ();
> 
> -   if (!unlimited_cost_model ())
> +   if (!unlimited_cost_model (LOOP_VINFO_LOOP (loop_vinfo)))
>       {
>         res.inside_cost = INT_MAX;
>         res.outside_cost = INT_MAX;
> @@ -1429,7 +1430,7 @@ vect_enhance_data_refs_alignment (loop_vec_info
> loop_vinfo)
>                   vectorization factor.
>                   We do this automtically for cost model, since we
> calculate cost
>                   for every peeling option.  */
> -              if (unlimited_cost_model ())
> +              if (unlimited_cost_model (LOOP_VINFO_LOOP (loop_vinfo)))
>                  possible_npeel_number = vf /nelements;
> 
>                /* Handle the aligned case. We may decide to align some other
> @@ -1437,7 +1438,7 @@ vect_enhance_data_refs_alignment (loop_vec_info
> loop_vinfo)
>                if (DR_MISALIGNMENT (dr) == 0)
>                  {
>                    npeel_tmp = 0;
> -                  if (unlimited_cost_model ())
> +                  if (unlimited_cost_model (LOOP_VINFO_LOOP (loop_vinfo)))
>                      possible_npeel_number++;
>                  }
> 
> diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
> index 86ebbd2..1fd28e3 100644
> --- a/gcc/tree-vect-loop.c
> +++ b/gcc/tree-vect-loop.c
> @@ -2696,7 +2696,7 @@ vect_estimate_min_profitable_iters
> (loop_vec_info loop_vinfo,
>    void *target_cost_data = LOOP_VINFO_TARGET_COST_DATA (loop_vinfo);
> 
>    /* Cost model disabled.  */
> -  if (unlimited_cost_model ())
> +  if (unlimited_cost_model (LOOP_VINFO_LOOP (loop_vinfo)))
>      {
>        dump_printf_loc (MSG_NOTE, vect_location, "cost model disabled.\n");
>        *ret_min_profitable_niters = 0;
> @@ -2929,6 +2929,13 @@ vect_estimate_min_profitable_iters
> (loop_vec_info loop_vinfo,
>    /* vector version will never be profitable.  */
>    else
>      {
> +      if (LOOP_VINFO_LOOP (loop_vinfo)->force_vect)
> +        {
> +          warning_at (LOOP_VINFO_LOC (loop_vinfo), OPT_Wopenmp_simd,
> +                      "Vectorization did not happen for "
> +                      "the loop labeled as simd.");
> +        }
> +
>        if (dump_enabled_p ())
>          dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>   "cost model: the vector iteration cost = %d "
> diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
> index 247bdfd..4b25964 100644
> --- a/gcc/tree-vect-slp.c
> +++ b/gcc/tree-vect-slp.c
> @@ -2171,7 +2171,7 @@ vect_slp_analyze_bb_1 (basic_block bb)
>      }
> 
>    /* Cost model: check if the vectorization is worthwhile.  */
> -  if (!unlimited_cost_model ()
> +  if (!unlimited_cost_model (NULL)
>        && !vect_bb_vectorization_profitable_p (bb_vinfo))
>      {
>        if (dump_enabled_p ())
> diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
> index a6c5b59..fd255db 100644
> --- a/gcc/tree-vectorizer.h
> +++ b/gcc/tree-vectorizer.h
> @@ -919,9 +919,12 @@ known_alignment_for_access_p (struct
> data_reference *data_ref_info)
> 
>  /* Return true if the vect cost model is unlimited.  */
>  static inline bool
> -unlimited_cost_model ()
> +unlimited_cost_model (loop_p loop)
>  {
> -  return flag_vect_cost_model == VECT_COST_MODEL_UNLIMITED;
> +  return (flag_vect_cost_model == VECT_COST_MODEL_UNLIMITED
> +          || (loop != NULL
> +              && loop->force_vect
> +              && flag_simd_cost_model == VECT_COST_MODEL_UNLIMITED));
>  }
> 
>  /* Source location */
> 
> 
> On Wed, Nov 20, 2013 at 12:14 AM, Tobias Burnus <burnus@net-b.de> wrote:
> > I have some small comments to the patch:
> >
> > * You should also update gcc/doc/invoke.texi
> >
> >
> > Sergey Ostanevich wrote:
> >>
> >> index 0026683..84911a0 100644
> >> --- a/gcc/c-family/c.opt
> >> +++ b/gcc/c-family/c.opt
> >
> > ...
> >
> >> +Wopenmp-simd
> >> +C C++ Var(openmp_simd) Warning
> >> +Warn about omp simd construct is overridden by cost model
> >
> >
> >> --- a/gcc/fortran/lang.opt
> >> +++ b/gcc/fortran/lang.opt
> >
> > ...
> >
> >> +Wopenmp-simd
> >> +Fortran Warning
> >> +Warn about omp simd construct is overridden by cost model
> >
> >
> > As the option files get merged, using
> >
> > Wopenmp-simd
> > Fortran
> > ; Documented in C
> >
> > is sufficient.
> >
> >
> >
> >> +fsimd-cost-model=
> >> +Common Joined RejectNegative Enum(vect_cost_model)
> >> Var(flag_simd_cost_model) Init(VECT_COST_MODEL_DEFAULT)
> >> +Specifies the cost model for vectorization in loops marked with omp simd
> >
> >
> > I wonder whether we need to care about Cilk Plus' "#pragma simd" in this
> > summary.
> >
> >
> >
> >
> >> +      if (LOOP_VINFO_LOOP (loop_vinfo)->force_vect)
> >> +        {
> >> +          warning (OPT_Wopenmp_simd, "Vectorization did not happen
> >> for the loop ",
> >> +                   "labeled as simd");
> >> +        }
> >> +
> >
> >
> > The warning line is too long.
> >
> >
> > Tobias
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer