Re: [PATCH] Fix ICE for boolean comparison

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

On Thu, Nov 12, 2015 at 4:44 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> Hi,
>
> Currently compiler may ICE when loaded boolean is compared with vector invariant or another boolean value.  This is because we don't detect mix of bool and non-bool vectypes and incorrectly determine vectype for boolean loop invariant for comparison.  This was fixed for COND_EXP before but also needs to be fixed for comparison.  This patch was bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for trunk?

Hmm, so this disables vectorization in these cases.  Isn't this a
regression?  Shouldn't we simply "materialize"
the non-bool vector from the boolean one say, with

 vec = boolvec ? {-1, -1 ... } : {0, 0, 0 ...}

?

Thanks,
Richard.

> Thanks,
> Ilya
> --
> gcc/
>
> 2015-11-12  Ilya Enkovich  <enkovich.gnu@gmail.com>
>
>         * tree-vect-loop.c (vect_determine_vectorization_factor): Check
>         mix of boolean and integer vectors in a single statement.
>         * tree-vect-slp.c (vect_mask_constant_operand_p): New.
>         (vect_get_constant_vectors): Use vect_mask_constant_operand_p to
>         determine constant type.
>         * tree-vect-stmts.c (vectorizable_comparison): Provide vectype
>         for loop invariants.
>
> gcc/testsuite/
>
> 2015-11-12  Ilya Enkovich  <enkovich.gnu@gmail.com>
>
>         * g++.dg/vect/simd-bool-comparison-1.cc: New test.
>         * g++.dg/vect/simd-bool-comparison-2.cc: New test.
>
>
> diff --git a/gcc/testsuite/g++.dg/vect/simd-bool-comparison-1.cc b/gcc/testsuite/g++.dg/vect/simd-bool-comparison-1.cc
> new file mode 100644
> index 0000000..a08362f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/vect/simd-bool-comparison-1.cc
> @@ -0,0 +1,21 @@
> +// { dg-do compile }
> +// { dg-additional-options "-mavx512bw -mavx512dq" { target { i?86-*-* x86_64-*-* } } }
> +
> +#define N 1024
> +
> +double a[N];
> +bool b[N];
> +bool c;
> +
> +void test ()
> +{
> +  int i;
> +
> +  for (i = 0; i < N; i++)
> +    if (b[i] != c)
> +      a[i] = 0.0;
> +    else
> +      a[i] = 1.0;
> +}
> +
> +// { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target { i?86-*-* x86_64-*-* } } } }
> diff --git a/gcc/testsuite/g++.dg/vect/simd-bool-comparison-2.cc b/gcc/testsuite/g++.dg/vect/simd-bool-comparison-2.cc
> new file mode 100644
> index 0000000..4accf56
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/vect/simd-bool-comparison-2.cc
> @@ -0,0 +1,20 @@
> +// { dg-do compile }
> +// { dg-additional-options "-mavx512bw -mavx512dq" { target { i?86-*-* x86_64-*-* } } }
> +
> +#define N 1024
> +
> +double a[N];
> +bool b[N];
> +char c[N];
> +
> +void test ()
> +{
> +  int i;
> +
> +  #pragma omp simd
> +  for (i = 0; i < N; i++)
> +    if ((c[i] > 0) && b[i])
> +      a[i] = 0.0;
> +    else
> +      a[i] = 1.0;
> +}
> diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
> index 55e5309..6b78b55 100644
> --- a/gcc/tree-vect-loop.c
> +++ b/gcc/tree-vect-loop.c
> @@ -649,7 +649,32 @@ vect_determine_vectorization_factor (loop_vec_info loop_vinfo)
>                     }
>                   return false;
>                 }
> +             else if (VECTOR_BOOLEAN_TYPE_P (mask_type)
> +                      != VECTOR_BOOLEAN_TYPE_P (vectype))
> +               {
> +                 if (dump_enabled_p ())
> +                   {
> +                     dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> +                                      "not vectorized: mixed mask and "
> +                                      "nonmask vector 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;
> +               }
>             }
> +
> +         /* We may compare boolean value loaded as vector of integers.
> +            Fix mask_type in such case.  */
> +         if (mask_type
> +             && !VECTOR_BOOLEAN_TYPE_P (mask_type)
> +             && gimple_code (stmt) == GIMPLE_ASSIGN
> +             && TREE_CODE_CLASS (gimple_assign_rhs_code (stmt)) == tcc_comparison)
> +           mask_type = build_same_sized_truth_vector_type (mask_type);
>         }
>
>        /* No mask_type should mean loop invariant predicate.
> diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
> index 9d97140..f3acb04 100644
> --- a/gcc/tree-vect-slp.c
> +++ b/gcc/tree-vect-slp.c
> @@ -2589,6 +2589,57 @@ vect_slp_bb (basic_block bb)
>  }
>
>
> +/* Return 1 if vector type of boolean constant which is OPNUM
> +   operand in statement STMT is a boolean vector.  */
> +
> +static bool
> +vect_mask_constant_operand_p (gimple *stmt, int opnum)
> +{
> +  stmt_vec_info stmt_vinfo = vinfo_for_stmt (stmt);
> +  enum tree_code code = gimple_expr_code (stmt);
> +  tree op, vectype;
> +  gimple *def_stmt;
> +  enum vect_def_type dt;
> +
> +  /* For comparison and COND_EXPR type is chosen depending
> +     on the other comparison operand.  */
> +  if (TREE_CODE_CLASS (code) == tcc_comparison)
> +    {
> +      if (opnum)
> +       op = gimple_assign_rhs1 (stmt);
> +      else
> +       op = gimple_assign_rhs2 (stmt);
> +
> +      if (!vect_is_simple_use (op, stmt_vinfo->vinfo, &def_stmt,
> +                              &dt, &vectype))
> +       gcc_unreachable ();
> +
> +      return !vectype || VECTOR_BOOLEAN_TYPE_P (vectype);
> +    }
> +
> +  if (code == COND_EXPR)
> +    {
> +      tree cond = gimple_assign_rhs1 (stmt);
> +
> +      if (TREE_CODE (cond) == SSA_NAME)
> +       return false;
> +
> +      if (opnum)
> +       op = TREE_OPERAND (cond, 1);
> +      else
> +       op = TREE_OPERAND (cond, 0);
> +
> +      if (!vect_is_simple_use (op, stmt_vinfo->vinfo, &def_stmt,
> +                              &dt, &vectype))
> +       gcc_unreachable ();
> +
> +      return !vectype || VECTOR_BOOLEAN_TYPE_P (vectype);
> +    }
> +
> +  return VECTOR_BOOLEAN_TYPE_P (STMT_VINFO_VECTYPE (stmt_vinfo));
> +}
> +
> +
>  /* For constant and loop invariant defs of SLP_NODE this function returns
>     (vector) defs (VEC_OPRNDS) that will be used in the vectorized stmts.
>     OP_NUM determines if we gather defs for operand 0 or operand 1 of the RHS of
> @@ -2625,8 +2676,7 @@ vect_get_constant_vectors (tree op, slp_tree slp_node,
>
>    /* Check if vector type is a boolean vector.  */
>    if (TREE_CODE (TREE_TYPE (op)) == BOOLEAN_TYPE
> -      && (VECTOR_BOOLEAN_TYPE_P (STMT_VINFO_VECTYPE (stmt_vinfo))
> -         || (code == COND_EXPR && op_num < 2)))
> +      && vect_mask_constant_operand_p (stmt, op_num))
>      vector_type
>        = build_same_sized_truth_vector_type (STMT_VINFO_VECTYPE (stmt_vinfo));
>    else
> diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
> index cfe30e0..c990dfa 100644
> --- a/gcc/tree-vect-stmts.c
> +++ b/gcc/tree-vect-stmts.c
> @@ -7642,8 +7642,8 @@ vectorizable_comparison (gimple *stmt, gimple_stmt_iterator *gsi,
>             }
>           else
>             {
> -             vec_rhs1 = vect_get_vec_def_for_operand (rhs1, stmt, NULL);
> -             vec_rhs2 = vect_get_vec_def_for_operand (rhs2, stmt, NULL);
> +             vec_rhs1 = vect_get_vec_def_for_operand (rhs1, stmt, vectype);
> +             vec_rhs2 = vect_get_vec_def_for_operand (rhs2, stmt, vectype);
>             }
>         }
>        else