Re: Vectorize stores with unknown stride

[Alan Lawrence <alan dot lawrence at arm dot com>]

(Below are all minor/style points only, no reason for patch not to go in.)

Michael Matz wrote:
> diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
> index 96afc7a..6d8f17e 100644
> --- a/gcc/tree-vect-data-refs.c
> +++ b/gcc/tree-vect-data-refs.c
> @@ -665,7 +665,7 @@ vect_compute_data_ref_alignment (struct data_reference *dr)
>
>    /* Strided loads perform only component accesses, misalignment information
>       is irrelevant for them.  */
> -  if (STMT_VINFO_STRIDE_LOAD_P (stmt_info))
> +  if (STMT_VINFO_STRIDED_P (stmt_info))
>      return true;
>    misalign = DR_INIT (dr);

Also update comment? (5 identical cases)

> @@ -2368,7 +2368,7 @@ vect_analyze_data_ref_access (struct data_reference *dr)
>
>    /* Assume this is a DR handled by non-constant strided load case.  */
>    if (TREE_CODE (step) != INTEGER_CST)
> -    return STMT_VINFO_STRIDE_LOAD_P (stmt_info);
> +    return STMT_VINFO_STRIDED_P (stmt_info);

Also update comment?

> diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
> index 2ce6d4d..d268eb0 100644
> --- a/gcc/tree-vect-stmts.c
> +++ b/gcc/tree-vect-stmts.c
> @@ -5013,7 +5025,7 @@ vectorizable_store (gimple stmt, gimple_stmt_iterator *gsi, gimple *vec_stmt,
>    tree dataref_ptr = NULL_TREE;
>    tree dataref_offset = NULL_TREE;
>    gimple ptr_incr = NULL;
> -  int nunits = TYPE_VECTOR_SUBPARTS (vectype);
> +  unsigned int nunits = TYPE_VECTOR_SUBPARTS (vectype);

Unrelated? (though I don't object to it!)

> @@ -5100,38 +5112,42 @@ vectorizable_store (gimple stmt, gimple_stmt_iterator *gsi, gimple *vec_stmt,
>    if (!STMT_VINFO_DATA_REF (stmt_info))
>      return false;
>
> -  negative =
> -    tree_int_cst_compare (loop && nested_in_vect_loop_p (loop, stmt)
> -                         ? STMT_VINFO_DR_STEP (stmt_info) : DR_STEP (dr),
> -                         size_zero_node) < 0;
> -  if (negative && ncopies > 1)
> -    {
> -      if (dump_enabled_p ())
> -        dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> -                        "multiple types with negative step.\n");
> -      return false;
> -    }
> -
> -  if (negative)
> +  if (STMT_VINFO_STRIDED_P (stmt_info))
> +    ;
> +  else

This is not a long chain of ifs, so is there a reason not to have
if (!STMT_VINFO_STRIDED_P (stmt_info))
?

> diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
> index 0796cc1..d231626 100644
> --- a/gcc/tree-vectorizer.h
> +++ b/gcc/tree-vectorizer.h
> @@ -661,7 +663,7 @@ typedef struct _stmt_vec_info {
>  #define STMT_VINFO_VECTORIZABLE(S)         (S)->vectorizable
>  #define STMT_VINFO_DATA_REF(S)             (S)->data_ref_info
>  #define STMT_VINFO_GATHER_P(S)            (S)->gather_p
> -#define STMT_VINFO_STRIDE_LOAD_P(S)       (S)->stride_load_p
> +#define STMT_VINFO_STRIDED_P(S)                   (S)->strided_p

Spacing (?)

>  #define STMT_VINFO_SIMD_LANE_ACCESS_P(S)   (S)->simd_lane_access_p
>
>  #define STMT_VINFO_DR_BASE_ADDRESS(S)      (S)->dr_base_address
> diff --git a/gcc/testsuite/gcc.dg/vect/vect-strided-store.c b/gcc/testsuite/gcc.dg/vect/vect-strided-store.c
> new file mode 100644
> index 0000000..32bcff9
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vect/vect-strided-store.c
> @@ -0,0 +1,30 @@
> +/* { dg-require-effective-target vect_float } */
> +
> +#include <stdarg.h>
> +#include "tree-vect.h"
> +
> +void __attribute__((noinline))
> +sumit (float * __restrict dest,
> +       float * __restrict src, float * __restrict src2,
> +       int stride, int n)
> +{
> +  int i;
> +  for (i = 0; i < n; i++)
> +    dest[i*stride] = src[i] + src2[i];
> +}
> +
> +int main()
> +{
> +  int i;
> +  float src[] = {1, 2, 3, 4, 5, 6, 7, 8};
> +  float dest[8];
> +  check_vect ();
> +  sumit (dest, src, src, 1, 8);
> +  for (i = 0; i < 8; i++)
> +    if (2*src[i] != dest[i])
> +      abort ();
> +  return 0;
> +}
> +
> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" } } */
> +/* { dg-final { cleanup-tree-dump "vect" } } */

While I appreciate the vectorizer doesn't know the invariant stride is going to 
be '1' when the loop executes...IMVHO I'd feel more reassured if we passed in 
stride>1 at runtime ;). In fact I'd feel more reassured still, if we did the 
thing with a few different values of stride...

Cheers, Alan