Re: [vec-cmp, patch 2/6] Vectorization factor computation

[Richard Biener <richard dot guenther at gmail dot com>]

On Thu, Oct 8, 2015 at 4:59 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> Hi,
>
> This patch handles statements with boolean result in vectorization factor computation.  For comparison its operands type is used instead of restult type to compute VF.  Other boolean statements are ignored for VF.
>
> Vectype for comparison is computed using type of compared values.  Computed type is propagated into other boolean operations.

This feels rather ad-hoc, mixing up the existing way of computing
vector type and VF.  I'd rather have turned the whole
vector type computation around to the scheme working on the operands
rather than on the lhs and then searching
for smaller/larger types on the rhs'.

I know this is a tricky function (heh, but you make it even worse...).
And it needs a helper with knowledge about operations
so one can compute the result vector type for an operation on its
operands.  The seeds should be PHIs (handled like now)
and loads, and yes, externals need special handling.

Ideally we'd do things in two stages, first compute vector types in a
less constrained manner (not forcing a single vector size)
and then in a 2nd run promote to a common size also computing the VF to do that.

Btw, I think you "mishandle" bool b = boolvar != 0;

Richard.

> Thanks,
> Ilya
> --
> gcc/
>
> 2015-10-08  Ilya Enkovich  <enkovich.gnu@gmail.com>
>
>         * tree-vect-loop.c (vect_determine_vectorization_factor):  Ignore mask
>         operations for VF.  Add mask type computation.
>         * tree-vect-stmts.c (get_mask_type_for_scalar_type): New.
>         * tree-vectorizer.h (get_mask_type_for_scalar_type): New.
>
>
> diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
> index 63e29aa..c7e8067 100644
> --- a/gcc/tree-vect-loop.c
> +++ b/gcc/tree-vect-loop.c
> @@ -183,19 +183,21 @@ vect_determine_vectorization_factor (loop_vec_info loop_vinfo)
>  {
>    struct loop *loop = LOOP_VINFO_LOOP (loop_vinfo);
>    basic_block *bbs = LOOP_VINFO_BBS (loop_vinfo);
> -  int nbbs = loop->num_nodes;
> +  unsigned nbbs = loop->num_nodes;
>    unsigned int vectorization_factor = 0;
>    tree scalar_type;
>    gphi *phi;
>    tree vectype;
>    unsigned int nunits;
>    stmt_vec_info stmt_info;
> -  int i;
> +  unsigned i;
>    HOST_WIDE_INT dummy;
>    gimple *stmt, *pattern_stmt = NULL;
>    gimple_seq pattern_def_seq = NULL;
>    gimple_stmt_iterator pattern_def_si = gsi_none ();
>    bool analyze_pattern_stmt = false;
> +  bool bool_result;
> +  auto_vec<stmt_vec_info> mask_producers;
>
>    if (dump_enabled_p ())
>      dump_printf_loc (MSG_NOTE, vect_location,
> @@ -414,6 +416,8 @@ vect_determine_vectorization_factor (loop_vec_info loop_vinfo)
>               return false;
>             }
>
> +         bool_result = false;
> +
>           if (STMT_VINFO_VECTYPE (stmt_info))
>             {
>               /* The only case when a vectype had been already set is for stmts
> @@ -434,6 +438,32 @@ vect_determine_vectorization_factor (loop_vec_info loop_vinfo)
>                 scalar_type = TREE_TYPE (gimple_call_arg (stmt, 3));
>               else
>                 scalar_type = TREE_TYPE (gimple_get_lhs (stmt));
> +
> +             /* Bool ops don't participate in vectorization factor
> +                computation.  For comparison use compared types to
> +                compute a factor.  */
> +             if (TREE_CODE (scalar_type) == BOOLEAN_TYPE)
> +               {
> +                 mask_producers.safe_push (stmt_info);
> +                 bool_result = true;
> +
> +                 if (gimple_code (stmt) == GIMPLE_ASSIGN
> +                     && TREE_CODE_CLASS (gimple_assign_rhs_code (stmt))
> +                        == tcc_comparison
> +                     && TREE_CODE (TREE_TYPE (gimple_assign_rhs1 (stmt)))
> +                        != BOOLEAN_TYPE)
> +                   scalar_type = TREE_TYPE (gimple_assign_rhs1 (stmt));
> +                 else
> +                   {
> +                     if (!analyze_pattern_stmt && gsi_end_p (pattern_def_si))
> +                       {
> +                         pattern_def_seq = NULL;
> +                         gsi_next (&si);
> +                       }
> +                     continue;
> +                   }
> +               }
> +
>               if (dump_enabled_p ())
>                 {
>                   dump_printf_loc (MSG_NOTE, vect_location,
> @@ -456,7 +486,8 @@ vect_determine_vectorization_factor (loop_vec_info loop_vinfo)
>                   return false;
>                 }
>
> -             STMT_VINFO_VECTYPE (stmt_info) = vectype;
> +             if (!bool_result)
> +               STMT_VINFO_VECTYPE (stmt_info) = vectype;
>
>               if (dump_enabled_p ())
>                 {
> @@ -469,8 +500,9 @@ vect_determine_vectorization_factor (loop_vec_info loop_vinfo)
>           /* The vectorization factor is according to the smallest
>              scalar type (or the largest vector size, but we only
>              support one vector size per loop).  */
> -         scalar_type = vect_get_smallest_scalar_type (stmt, &dummy,
> -                                                      &dummy);
> +         if (!bool_result)
> +           scalar_type = vect_get_smallest_scalar_type (stmt, &dummy,
> +                                                        &dummy);
>           if (dump_enabled_p ())
>             {
>               dump_printf_loc (MSG_NOTE, vect_location,
> @@ -545,6 +577,100 @@ vect_determine_vectorization_factor (loop_vec_info loop_vinfo)
>      }
>    LOOP_VINFO_VECT_FACTOR (loop_vinfo) = vectorization_factor;
>
> +  for (i = 0; i < mask_producers.length (); i++)
> +    {
> +      tree mask_type = NULL;
> +      bb_vec_info bb_vinfo = STMT_VINFO_BB_VINFO (mask_producers[i]);
> +
> +      stmt = STMT_VINFO_STMT (mask_producers[i]);
> +
> +      if (gimple_code (stmt) == GIMPLE_ASSIGN
> +         && TREE_CODE_CLASS (gimple_assign_rhs_code (stmt)) == tcc_comparison
> +         && TREE_CODE (TREE_TYPE (gimple_assign_rhs1 (stmt))) != BOOLEAN_TYPE)
> +       {
> +         scalar_type = TREE_TYPE (gimple_assign_rhs1 (stmt));
> +         mask_type = get_mask_type_for_scalar_type (scalar_type);
> +
> +         if (!mask_type)
> +           {
> +             if (dump_enabled_p ())
> +               dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> +                                "not vectorized: unsupported mask\n");
> +             return false;
> +           }
> +       }
> +      else
> +       {
> +         tree rhs, def;
> +         ssa_op_iter iter;
> +         gimple *def_stmt;
> +         enum vect_def_type dt;
> +
> +         FOR_EACH_SSA_TREE_OPERAND (rhs, stmt, iter, SSA_OP_USE)
> +           {
> +             if (!vect_is_simple_use_1 (rhs, stmt, loop_vinfo, bb_vinfo,
> +                                        &def_stmt, &def, &dt, &vectype))
> +               {
> +                 if (dump_enabled_p ())
> +                   {
> +                     dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> +                                      "not vectorized: can't compute mask type "
> +                                      "for statement, ");
> +                     dump_gimple_stmt (MSG_MISSED_OPTIMIZATION,  TDF_SLIM, stmt,
> +                                       0);
> +                     dump_printf (MSG_MISSED_OPTIMIZATION, "\n");
> +                   }
> +                 return false;
> +               }
> +
> +             /* No vectype probably means external definition.
> +                Allow it in case there is another operand which
> +                allows to determine mask type.  */
> +             if (!vectype)
> +               continue;
> +
> +             if (!mask_type)
> +               mask_type = vectype;
> +             else if (TYPE_VECTOR_SUBPARTS (mask_type)
> +                      != TYPE_VECTOR_SUBPARTS (vectype))
> +               {
> +                 if (dump_enabled_p ())
> +                   {
> +                     dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> +                                      "not vectorized: different sized masks "
> +                                      "types in statement, ");
> +                     dump_generic_expr (MSG_MISSED_OPTIMIZATION, TDF_SLIM,
> +                                        mask_type);
> +                     dump_printf (MSG_MISSED_OPTIMIZATION, " and ");
> +                     dump_generic_expr (MSG_MISSED_OPTIMIZATION, TDF_SLIM,
> +                                        vectype);
> +                     dump_printf (MSG_MISSED_OPTIMIZATION, "\n");
> +                   }
> +                 return false;
> +               }
> +           }
> +       }
> +
> +      /* No mask_type should mean loop invariant predicate.
> +        This is probably a subject for optimization in
> +        if-conversion.  */
> +      if (!mask_type)
> +       {
> +         if (dump_enabled_p ())
> +           {
> +             dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> +                              "not vectorized: can't compute mask type "
> +                              "for statement, ");
> +             dump_gimple_stmt (MSG_MISSED_OPTIMIZATION,  TDF_SLIM, stmt,
> +                               0);
> +             dump_printf (MSG_MISSED_OPTIMIZATION, "\n");
> +           }
> +         return false;
> +       }
> +
> +      STMT_VINFO_VECTYPE (mask_producers[i]) = mask_type;
> +    }
> +
>    return true;
>  }
>
> diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
> index e93f5ef..8eda8e9 100644
> --- a/gcc/tree-vect-stmts.c
> +++ b/gcc/tree-vect-stmts.c
> @@ -8190,6 +8190,23 @@ get_vectype_for_scalar_type (tree scalar_type)
>    return vectype;
>  }
>
> +/* Function get_mask_type_for_scalar_type.
> +
> +   Returns the mask type corresponding to a result of comparison
> +   of vectors of specified SCALAR_TYPE as supported by target.  */
> +
> +tree
> +get_mask_type_for_scalar_type (tree scalar_type)
> +{
> +  tree vectype = get_vectype_for_scalar_type (scalar_type);
> +
> +  if (!vectype)
> +    return NULL;
> +
> +  return build_truth_vector_type (TYPE_VECTOR_SUBPARTS (vectype),
> +                                 current_vector_size);
> +}
> +
>  /* Function get_same_sized_vectype
>
>     Returns a vector type corresponding to SCALAR_TYPE of size
> diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
> index e4d1feb..5d8e945 100644
> --- a/gcc/tree-vectorizer.h
> +++ b/gcc/tree-vectorizer.h
> @@ -995,6 +995,7 @@ extern bool vect_can_advance_ivs_p (loop_vec_info);
>  /* In tree-vect-stmts.c.  */
>  extern unsigned int current_vector_size;
>  extern tree get_vectype_for_scalar_type (tree);
> +extern tree get_mask_type_for_scalar_type (tree);
>  extern tree get_same_sized_vectype (tree, tree);
>  extern bool vect_is_simple_use (tree, gimple *, loop_vec_info,
>                                 bb_vec_info, gimple **,