Re: [patch, vectorizer] Fix PR tree-optimization/33866 ICE in vect_get_vec_def_for_stmt_copy

["Richard Guenther" <richard dot guenther at gmail dot com>]

On 10/24/07, Ira Rosen <IRAR@il.ibm.com> wrote:
>
> Hi,
>
> When we vectorize a group of stores with strided access in a loop with
> multiple types, we gather all the scalar operands of the stores in the
> group and later call vect_get_vec_def_for_stmt_copy () to get vectorized
> copies for the scalar operands. We (erroneously) assumed that the def stmts
> of those operands must be of the same type, i.e., either they all are loop
> invariants or loop variants.
> But in the testcase in PR 33866 this is not the case:
>
>   points[num_points_36][0] = start$0_20(D);
>   D.1561_14 = (long int) j_35;
>   D.1563_16 = D.1561_14 + start$1_19(D);
>   points[num_points_36][1] = D.1563_16;
>
> start$0_20(D) is loop invariant, while D.1563_16 is not.
>
> This patch checks the type of the def stmt of each operand in the group and
> calls vect_get_vec_def_for_stmt_copy () with the correct def stmt type.
>
> Bootstrapped with vectorization enabled and now testing on x86_64-linux.
> O.K. for mainline once the testing is completed?

Yes, ok with the fixed testcase.

Thanks,
Richard.

> Thanks,
> Ira
>
> ChangeLog:
>
>       PR tree-optimization/33866
>       * tree-vect-transform.c (vectorizable_store): Check operands of all
> the
>       stmts in the group of strided accesses. Get def stmt type for each
> store
>       in the group and pass it to vect_get_vec_def_for_stmt_copy ().
>
> testsuite/ChangeLog:
>
>       PR tree-optimization/33866
>       * gcc.dg/vect/pr33866.c: New testcase.
>
>
> Index: tree-vect-transform.c
> ===================================================================
> --- tree-vect-transform.c       (revision 129599)
> +++ tree-vect-transform.c       (working copy)
> @@ -4578,7 +4578,7 @@ vectorizable_store (tree stmt, block_stm
>    int nunits = TYPE_VECTOR_SUBPARTS (vectype);
>    int ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits;
>    int j;
> -  tree next_stmt, first_stmt;
> +  tree next_stmt, first_stmt = NULL_TREE;
>    bool strided_store = false;
>    unsigned int group_size, i;
>    VEC(tree,heap) *dr_chain = NULL, *oprnds = NULL, *result_chain = NULL;
> @@ -4640,9 +4640,28 @@ vectorizable_store (tree stmt, block_stm
>    if (STMT_VINFO_STRIDED_ACCESS (stmt_info))
>      {
>        strided_store = true;
> +      first_stmt = DR_GROUP_FIRST_DR (stmt_info);
>        if (!vect_strided_store_supported (vectype)
>           && !PURE_SLP_STMT (stmt_info) && !slp)
> -       return false;
> +       return false;
> +
> +      if (first_stmt == stmt)
> +       {
> +          /* STMT is the leader of the group. Check the operands of all
> the
> +             stmts of the group.  */
> +          next_stmt = DR_GROUP_NEXT_DR (stmt_info);
> +          while (next_stmt)
> +            {
> +              op = GIMPLE_STMT_OPERAND (next_stmt, 1);
> +              if (!vect_is_simple_use (op, loop_vinfo, &def_stmt, &def,
> &dt))
> +                {
> +                  if (vect_print_dump_info (REPORT_DETAILS))
> +                    fprintf (vect_dump, "use not simple.");
> +                  return false;
> +                }
> +              next_stmt = DR_GROUP_NEXT_DR (vinfo_for_stmt (next_stmt));
> +            }
> +        }
>      }
>
>    if (!vec_stmt) /* transformation not required.  */
> @@ -4657,7 +4676,6 @@ vectorizable_store (tree stmt, block_stm
>
>    if (strided_store)
>      {
> -      first_stmt = DR_GROUP_FIRST_DR (stmt_info);
>        first_dr = STMT_VINFO_DATA_REF (vinfo_for_stmt (first_stmt));
>        group_size = DR_GROUP_SIZE (vinfo_for_stmt (first_stmt));
>
> @@ -4803,8 +4821,9 @@ vectorizable_store (tree stmt, block_stm
>              OPRNDS are of size 1.  */
>           for (i = 0; i < group_size; i++)
>             {
> -             vec_oprnd = vect_get_vec_def_for_stmt_copy (dt,
> -                                                  VEC_index (tree, oprnds,
> i));
> +             op = VEC_index (tree, oprnds, i);
> +             vect_is_simple_use (op, loop_vinfo, &def_stmt, &def, &dt);
> +             vec_oprnd = vect_get_vec_def_for_stmt_copy (dt, op);
>               VEC_replace(tree, dr_chain, i, vec_oprnd);
>               VEC_replace(tree, oprnds, i, vec_oprnd);
>             }
> Index: testsuite/gcc.dg/vect/pr33866.c
> ===================================================================
> --- testsuite/gcc.dg/vect/pr33866.c     (revision 0)
> +++ testsuite/gcc.dg/vect/pr33866.c     (revision 0)
> @@ -0,0 +1,30 @@
> +/* Testcase by Martin Michlmayr <tbm@cyrius.com> */
> +
> +typedef struct
> +{
> +  long *coords;
> +}
> +fill_iter_info;
> +
> +extern H5Diterate (fill_iter_info *);
> +
> +void test_select_fill_hyper_simple (long *offset)
> +{
> +  long start[2];
> +  int num_points;
> +  long points[16][2];
> +  fill_iter_info iter_info;
> +  int i, j;
> +  iter_info.coords = (long *) points;
> +  for (i = 0, num_points = 0; j < (int) start[1]; j++, num_points++)
> +  {
> +    points[num_points][0] = i + start[0];
> +    points[num_points][1] = j + start[1];
> +  }
> +  H5Diterate (&iter_info);
> +}
> +
> +/* Needs interleaving support.  */
> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" {
> target { vect_interleave } } } } */
> +/* { dg-final { cleanup-tree-dump "vect" } } */
> +
>
>