Re: [patch, vectorizer] Fix PR tree-optimization/36493

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

On Thu, Jun 12, 2008 at 9:02 AM, Ira Rosen <IRAR@il.ibm.com> wrote:
>
> Hi,
>
> For the statement x[i] = i, where x is long and i is int, the vectorizer
> created vector statement with both sides of type vector int, which caused
> alias analysis to decide that  vector int * and long accesses do not
> conflict. This happened because for stores the vectorizer uses the type of
> rhs to create statements with operands of the same type
> (http://gcc.gnu.org/ml/gcc-patches/2006-10/msg00892.html).
>
> This patch adds a check that the types of the operands do not require  type
> conversion and removes the use of the rhs type.
>
> Bootstrapped and now testing on powerpc64-suse-linux. O.K. for 4.3.2 and
> 4.4 when the testing completes?
>
> Thanks,
> Ira
>
> ChangeLog:
>
>      PR tree-optimization/36493
>      * tree-vect-transform.c (vect_create_data_ref_ptr): Remove TYPE from
>      the arguments list. Use VECTYPE to create vector pointer.
>      (vectorizable_store): Fail if the operands are of different types.
>      Call vect_create_data_ref_ptr without the removed argument.
>      (vect_setup_realignment): Call vect_create_data_ref_ptr without the
>      removed argument.
>      (vectorizable_load): Likewise.
>
> testsuite/ChangeLog::
>
>      PR tree-optimization/36493
>      * gcc.dg/vect/pr36493.c: New testcase.
>
> Index: tree-vect-transform.c
> ===================================================================
> --- tree-vect-transform.c       (revision 136612)
> +++ tree-vect-transform.c       (working copy)
> @@ -49,7 +49,7 @@ along with GCC; see the file COPYING3.
>  static bool vect_transform_stmt (tree, block_stmt_iterator *, bool *,
> slp_tree);
>  static tree vect_create_destination_var (tree, tree);
>  static tree vect_create_data_ref_ptr
> -  (tree, struct loop*, tree, tree *, tree *, bool, tree, bool *);
> +  (tree, struct loop*, tree, tree *, tree *, bool, bool *);
>  static tree vect_create_addr_base_for_vector_ref
>   (tree, tree *, tree, struct loop *);
>  static tree vect_get_new_vect_var (tree, enum vect_var_kind, const char
> *);
> @@ -951,7 +951,6 @@ vect_create_addr_base_for_vector_ref (tr,
>         by the data-ref in STMT.
>    4. ONLY_INIT: indicate if vp is to be updated in the loop, or remain
>         pointing to the initial address.
> -   5. TYPE: if not NULL indicates the required type of the data-ref
>
>    Output::
>    1. Declare a new ptr to vector_type, and have it point to the base of
> the
> @@ -981,7 +980,7 @@ vect_create_addr_base_for_vector_ref (tr
>  static tree
>  vect_create_data_ref_ptr (tree stmt, struct loop *at_loop,
>                          tree offset, tree *initial_address, tree
> *ptr_incr,
> -                         bool only_init, tree type, bool *inv_p)
> +                         bool only_init, bool *inv_p)
>  {
>   tree base_name;
>   stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
> @@ -1040,10 +1039,8 @@ vect_create_data_ref_ptr (tree stmt, str
>     }
>
>   /** (1) Create the new vector-pointer variable:  **/
> -  if (type)
> -    vect_ptr_type = build_pointer_type (type);
> -  else
> -    vect_ptr_type = build_pointer_type (vectype);
> +  vect_ptr_type = build_pointer_type (vectype););
> +
>   vect_ptr = vect_get_new_vect_var (vect_ptr_type, vect_pointer_var,
>                                     get_name (base_name));
>   add_referenced_var (vect_ptr);
> @@ -4756,6 +4753,13 @@ vectorizable_store (tree stmt, block_stm
>       return false;
>     }
>
> +  if (!useless_type_conversion_p (TREE_TYPE (op), TREE_TYPE
> (scalar_dest))),
> +    {
> +      if (vect_print_dump_info (REPORT_DETAILS)),
> +        fprintf (vect_dump, "operands of different types");
> +      return false;
> +    }

This is always true.  In a GIMPLE_MODIFY_STMT the rhs type is
trivially convertible to the lhs type.

What you want to check is whether accesses through the vector pointer
correctly alias the scalar destination.  Thus something along the line of

  /* If accesses through a pointer to vectype do not alias the original
     memory reference we have a problem.  */
  if (get_alias_set (vectype) != get_alias_set (scalar_dest)
      && !alias_set_subset_of (get_alias_set (vectype), get_alias_set
(scalar_dest))
    {
      if (vect_print_dump_info (REPORT_DETAILS)),
        fprintf (vect_dump, "vector type does not alias scalar type");
      return false;
    }

which you in the end could fix by generating a ref-all pointer type in
vect_create_data_ref_ptr (using build_pointer_type_for_mode).

Otherwise the change looks ok.

Richard.

>   vec_mode = TYPE_MODE (vectype);
>   /* FORNOW. In some cases can vectorize even if data-type not
> supported))),
>      (e.g. - array initialization with 0).  */
> @@ -4930,9 +4934,10 @@ vectorizable_store (tree stmt, block_stm
>                  next_stmt = DR_GROUP_NEXT_DR (vinfo_for_stmt
> (next_stmt));,
>                }
>            }
> +
>          dataref_ptr = vect_create_data_ref_ptr (first_stmt, NULL,
> NULL_TREE,
> -                                                 &dummy, &ptr_incr, false,
> -                                                 TREE_TYPE (vec_oprnd),
> &inv_p);
> +                                                 &dummy, &ptr_incr, false,
> +                                                 &inv_p);
>          gcc_assert (!inv_p);
>        }
>       else
> @@ -5170,7 +5175,7 @@ vect_setup_realignment (tree stmt, block
>       pe = loop_preheader_edge (loop_for_initial_load);
>       vec_dest = vect_create_destination_var (scalar_dest, vectype);
>       ptr = vect_create_data_ref_ptr (stmt, loop_for_initial_load,
> NULL_TREE,
> -                               &init_addr, &inc, true, NULL_TREE, &inv_p);
> +                                     &init_addr, &inc, true, &inv_p);
>       data_ref = build1 (ALIGN_INDIRECT_REF, vectype, ptr);,
>       new_stmt = build_gimple_modify_stmt (vec_dest, data_ref);
>       new_temp = make_ssa_name (vec_dest, new_stmt);
> @@ -5811,7 +5816,7 @@ vectorizable_load (tree stmt, block_stmt);
>         dataref_ptr = vect_create_data_ref_ptr (first_stmt,
>                                                at_loop, offset,
>                                                &dummy, &ptr_incr, false,);
> -                                               NULL_TREE, &inv_p);
> +                                               &inv_p);
>       else
>         dataref_ptr =
>                bump_vector_ptr (dataref_ptr, ptr_incr, bsi, stmt,
> NULL_TREE);
> Index: testsuite/gcc.dg/vect/pr36493.c
> ===================================================================
> --- testsuite/gcc.dg/vect/pr36493.c     (revision 0)
> +++ testsuite/gcc.dg/vect/pr36493.c     (revision 0)
> @@ -0,0 +1,23 @@
> +/* { dg-require-effective-target vect_int } */
> +
> +#include "tree-vect.h"
> +
> +int
> +main (void)
> +{
> +  int i;
> +  long x[12] __attribute__((aligned(16)));}
> +
> +  x[0] = 1;]
> +  for (i = 0; i < 12; i++)
> +    x[i] = i;
> +
> +  if (x[0] != 0)
> +    abort ();
> +
> +  return 0;);
> +}
> +}
> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect"  } } */
> +/* { dg-final { cleanup-tree-dump "vect" } } */
> +
>
>