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

[Tobias Burnus <burnus at net-b dot de>]

Sergey,

Thanks for the modifications and the patch. I tried your patch using 
gcc/testsuite/gcc.dg/vect/costmodel/x86_64/costmodel-vect-31.c with the 
following change:

--- a/gcc/testsuite/gcc.dg/vect/costmodel/x86_64/costmodel-vect-31.c
+++ b/gcc/testsuite/gcc.dg/vect/costmodel/x86_64/costmodel-vect-31.c
@@ -27,2 +27,3 @@ int main1 ()
   /* unaligned */
+#pragma omp simd
   for (i = 0; i < N/2; i++)

The result is with and without "pragma omp simd" in effect is the 
following for
  gcc -fdump-tree-vect-details -fopt-info-vec-optimized -c -O2 
-ftree-vectorize -fvect-cost-model=dynamic costmodel-vect-31.c
and
  gcc -fdump-tree-vect-details -fopt-info-vec-optimized -c -O2 
-ftree-vectorize -fvect-cost-model=dynamic -fsimd-cost-model=default 
-Wopenmp-simd -fopenmp-simd costmodel-vect-31.c

costmodel-vect-31.c:68:3: note: loop vectorized
costmodel-vect-31.c:68:3: note: loop peeled for vectorization to enhance 
alignment
costmodel-vect-31.c:55:3: note: loop vectorized
costmodel-vect-31.c:42:3: note: loop vectorized

Namely, the loop in line 29-32 is not vectorized. But also: 
-Wopenmp-simd doesn't warn!

If one now enables -fsimd-cost-model=unlimited, one gets the loop 
vectorized:

costmodel-vect-31.c:68:3: note: loop vectorized
costmodel-vect-31.c:68:3: note: loop peeled for vectorization to enhance 
alignment
costmodel-vect-31.c:55:3: note: loop vectorized
costmodel-vect-31.c:42:3: note: loop vectorized
costmodel-vect-31.c:31:16: note: loop vectorized
costmodel-vect-31.c:31:16: note: loop peeled for vectorization to 
enhance alignment


Thus, can you check why the warning doesn't work? Additionally, I'd find 
is useful to have a test case for both -Wopenmp-simd and for 
-fsimd-cost-model=unlimited.


Below, a few minor remarks to your patch.

[If possibly, try to attach the patch in text format (e.g. Content-Type: 
text/plain) instead of binary (Content-Type: application/octet-stream) 
that makes reviewing the patch in the email program a bit easier.]

Sergey Ostanevich wrote:
> 2013-11-25  sergey.y.ostanevich  <sergos.gnu@gmail.com>
>
> 	* gcc/c-family/c.opt: Introduced a new openmp-simd warning.
> 	* gcc/fortran/lang.opt: Ditto.
> 	* gcc/common.opt: Introduced a new option -fsimd-cost-model.
> 	* gcc/doc/invoke.texi: Introduced a new openmp-simd warning and
> 	a new -fsimd-cost-model option.
> 	* gcc/tree-vectorizer.h (unlimited_cost_model): Interface updated
> 	to rely on the particular loop info.
> 	* gcc/tree-vect-data-refs.c (vect_peeling_hash_insert): Ditto.
> 	(vect_peeling_hash_choose_best_peeling): Ditto.
> 	(vect_enhance_data_refs_alignment): Ditto.
> 	* gcc/tree-vect-slp.c (vect_slp_analyze_bb_1): Ditto.
> 	* gcc/tree-vect-loop.c (vect_estimate_min_profitable_iters): Ditto
> 	plus added openmp-simd warining.

s/warining/warning/

Additionally, recall that gcc/, gcc/c-family and gcc/fortran have their 
own ChangeLog. Thus, at the end, the path names in those files should be 
relative to the relevant ChangeLog file ("c.opt", "lang.opt", 
"common.opt", "doc/invoke.texi" etc.)

> +@item -fsimd-cost-model=@var{model}
> ... The
> +@code{default} model means to reuse model defined by @option{fvect-cost-model}.
> +All other values of @var{model} have the same meaning as described in
> +@option{fvect-cost-model}.

I think it should be "to reuse the model" (with "the") and (twice) 
"-fvect..." instead of "fvect..." (i.e. with hyphen).

> @@ -2936,6 +2936,10 @@ vect_estimate_min_profitable_iters (loop_vec_info loop_vinfo,
>     /* vector version will never be profitable.  */
>     else
>       {
> +      if (warn_openmp_simd && LOOP_VINFO_LOOP (loop_vinfo)->force_vect)
> +	warning_at (vect_location, OPT_Wopenmp_simd, "vectorization "
> +		    "did not happen for a simd loop");

As the example shows, this code is seemingly not reached.

Otherwise, the code looks fine to me (although, I cannot approve 
anything but GCC's Fortran part).

Tobias