Re: [PATCH 1/2][vect]PR 88915: Vectorize epilogues when versioning loops

[Jeff Law <law at redhat dot com>]

On 8/23/19 10:50 AM, Andre Vieira (lists) wrote:
> Hi,
> 
> This patch is an improvement on my last RFC.  As you pointed out, we can
> do the vectorization analysis of the epilogues before doing the
> transformation, using the same approach as used by openmp simd.  I have
> not yet incorporated the cost tweaks for vectorizing the epilogue, I
> would like to do this in a subsequent patch, to make it easier to test
> the differences.
> 
> I currently disable the vectorization of epilogues when versioning for
> iterations.  This is simply because I do not completely understand how
> the assumptions are created and couldn't determine whether using
> skip_vectors with this would work.  If you don't think it is a problem
> or have a testcase to show it work I would gladly look at it.
> 
> Bootstrapped this and the next patch on x86_64 and
> aarch64-unknown-linux-gnu, with no regressions (after test changes in
> next patch).
> 
> gcc/ChangeLog:
> 2019-08-23  Andre Vieira  <andre.simoesdiasvieira@arm.com>
> 
>          PR 88915
>          * gentype.c (main): Add poly_uint64 type to generator.
>          * tree-vect-loop.c (vect_analyze_loop_2): Make it determine
>          whether we vectorize epilogue loops.
>          (vect_analyze_loop): Idem.
>          (vect_transform_loop): Pass decision to vectorize epilogues
>          to vect_do_peeling.
>          * tree-vect-loop-manip.c (vect_do_peeling): Enable skip-vectors
>          when doing loop versioning if we decided to vectorize epilogues.
>          (vect-loop_versioning): Moved decision to check_profitability
>          based on cost model.
>          * tree-vectorizer.h (vect_loop_versioning, vect_do_peeling,
>          vect_analyze_loop, vect_transform_loop): Update declarations.
>          * tree-vectorizer.c: Include params.h
>          (try_vectorize_loop_1): Initialize vect_epilogues_nomask
>          to PARAM_VECT_EPILOGUES_NOMASK and pass it to vect_analyze_loop
>          and vect_transform_loop.  Also make sure vectorizing epilogues
>          does not count towards number of vectorized loops.
Nit.  In several places you use "epilog", proper spelling is "epilogue".



> diff --git a/gcc/tree-vectorizer.c b/gcc/tree-vectorizer.c
> index 173e6b51652fd023893b38da786ff28f827553b5..25c3fc8ff55e017ae0b971fa93ce8ce2a07cb94c 100644
> --- a/gcc/tree-vectorizer.c
> +++ b/gcc/tree-vectorizer.c
> @@ -1013,8 +1015,13 @@ try_vectorize_loop_1 (hash_table<simduid_to_vf> *&simduid_to_vf_htab,
>  
>    /* Epilogue of vectorized loop must be vectorized too.  */
>    if (new_loop)
> -    ret |= try_vectorize_loop_1 (simduid_to_vf_htab, num_vectorized_loops,
> -				 new_loop, loop_vinfo, NULL, NULL);
> +    {
> +      /* Don't include vectorized epilogues in the "vectorized loops" count.
> +       */
> +      unsigned dont_count = *num_vectorized_loops;
> +      ret |= try_vectorize_loop_1 (simduid_to_vf_htab, &dont_count,
> +				   new_loop, loop_vinfo, NULL, NULL);
> +    }
Nit.  Don't wrap a comment with just the closing */ on its own line.
Instead wrap before "count" so that.

This is fine for the trunk after fixing those nits.

jeff