| Message ID | CAGYS_TJFF6we6wOpsVYSu_3iqKXL8WfEwYnkcmTrGMpiUrBpVg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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 > >
On Wed, Nov 20, 2013 at 02:59:21PM +0100, Richard Biener wrote: > > --- 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) Please use Var(warn_openmp_simd) here. > > --- 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) I'd say you want to add EnumValue Enum(vect_cost_model) String(default) Value(VECT_COST_MODEL_DEFAULT) here. > > @@ -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."); No {} around single stmt then body. Also, diagnostic messages don't start with a capital letter and don't end with dot. So "vectorization did not happen for " "a simd loop" or so. > > /* 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)); > > } IMNSHO this should instead do: if (loop != NULL && loop->force_vect && flag_simd_cost_model != VECT_COST_MODEL_DEFAULT) return flag_simd_cost_model == VECT_COST_MODEL_UNLIMITED; return flag_vect_cost_model == VECT_COST_MODEL_UNLIMITED; so, if user said that -fsimd-cost-model=default, then it should honor -fvect-cost-model. And, IMHO that should be the default, but I don't feel strongly about that. Jakub
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)); }
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? 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. /* 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