diff mbox

[vec-cmp,5/6] Disable bool patterns when possible

Message ID 20151022160401.GA23452@msticlxl57.ims.intel.com
State New
Headers show

Commit Message

Ilya Enkovich Oct. 22, 2015, 4:04 p.m. UTC
On 21 Oct 11:45, Jeff Law wrote:
> On 10/08/2015 09:15 AM, Ilya Enkovich wrote:
> >Hi,
> >
> >This patch disables transformation of boolean computations into integer ones in case target supports vector comparison.  Pattern still applies to transform resulting boolean value into integer or avoid COND_EXPR with SSA_NAME as condition.
> >
> >Thanks,
> >Ilya
> >--
> >2015-10-08  Ilya Enkovich  <enkovich.gnu@gmail.com>
> >
> >	* tree-vect-patterns.c (check_bool_pattern): Check fails
> >	if we can vectorize comparison directly.
> >	(search_type_for_mask): New.
> >	(vect_recog_bool_pattern): Support cases when bool pattern
> >	check fails.
> >
> >
> >diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
> >index 830801a..e3be3d1 100644
> >--- a/gcc/tree-vect-patterns.c
> >+++ b/gcc/tree-vect-patterns.c
> >@@ -2962,6 +2962,11 @@ check_bool_pattern (tree var, loop_vec_info loop_vinfo, bb_vec_info bb_vinfo)
> >  	  if (comp_vectype == NULL_TREE)
> >  	    return false;
> >
> >+	  mask_type = get_mask_type_for_scalar_type (TREE_TYPE (rhs1));
> >+	  if (mask_type
> >+	      && expand_vec_cmp_expr_p (comp_vectype, mask_type))
> >+	    return false;
> >+
> >  	  if (TREE_CODE (TREE_TYPE (rhs1)) != INTEGER_TYPE)
> >  	    {
> >  	      machine_mode mode = TYPE_MODE (TREE_TYPE (rhs1));
> So we're essentially saying here that we've got another preferred method for
> optimizing this case, right?
> 
> Can you update the function comment for check_bool_pattern?  In particular
> change the "if bool VAR can ..." to "can and should".
> 
> I think that more clearly states the updated purpose of that code.
> 
> 
> 
> 
> >@@ -3186,6 +3191,75 @@ adjust_bool_pattern (tree var, tree out_type, tree trueval,
> >  }
> >
> >
> >+/* Try to determine a proper type for converting bool VAR
> >+   into an integer value.  The type is chosen so that
> >+   conversion has the same number of elements as a mask
> >+   producer.  */
> >+
> >+static tree
> >+search_type_for_mask (tree var, loop_vec_info loop_vinfo, bb_vec_info bb_vinfo)
> What is the return value here?  Presumably the type or NULL.
> 
> So instead of "Try to determine a proper type" how about
> "Return the proper type or NULL_TREE if no such type exists ..."?
> 
> Please change the references to NULL to instead use NULL_TREE in that
> function as well.  They're functionally equivalent, but the latter is
> considered more correct these days.
> 
> 
> 
> >+	{
> >+	  tree type = search_type_for_mask (var, loop_vinfo, bb_vinfo);
> >+	  tree cst0, cst1, cmp, tmp;
> >+
> >+	  if (!type)
> >+	    return NULL;
> >+
> >+	  /* We may directly use cond with narrowed type to avoid
> >+	     multiple cond exprs with following result packing and
> >+	     perform single cond with packed mask intead.  In case
> s/intead/instead/
> 
> With those changes above, this should be OK for the trunk.
> 
> jeff
> 

Thanks for review!  Here is an updated version with all mentioned issues fixed.

Thanks,
Ilya
--
2015-09-12  Ilya Enkovich  <enkovich.gnu@gmail.com>

	* tree-vect-patterns.c (check_bool_pattern): Check fails
	if we can vectorize comparison directly.
	(search_type_for_mask): New.
	(vect_recog_bool_pattern): Support cases when bool pattern
	check fails.

Comments

Jeff Law Oct. 22, 2015, 4:35 p.m. UTC | #1
On 10/22/2015 10:04 AM, Ilya Enkovich wrote:

> 2015-09-12  Ilya Enkovich  <enkovich.gnu@gmail.com>
>
> 	* tree-vect-patterns.c (check_bool_pattern): Check fails
> 	if we can vectorize comparison directly.
> 	(search_type_for_mask): New.
> 	(vect_recog_bool_pattern): Support cases when bool pattern
> 	check fails.
OK to commit when its prerequisites (if any) are committed.

jeff
Richard Biener May 25, 2016, 11:37 a.m. UTC | #2
On Thu, Oct 22, 2015 at 6:04 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> On 21 Oct 11:45, Jeff Law wrote:
>> On 10/08/2015 09:15 AM, Ilya Enkovich wrote:
>> >Hi,
>> >
>> >This patch disables transformation of boolean computations into integer ones in case target supports vector comparison.  Pattern still applies to transform resulting boolean value into integer or avoid COND_EXPR with SSA_NAME as condition.
>> >
>> >Thanks,
>> >Ilya
>> >--
>> >2015-10-08  Ilya Enkovich  <enkovich.gnu@gmail.com>
>> >
>> >     * tree-vect-patterns.c (check_bool_pattern): Check fails
>> >     if we can vectorize comparison directly.
>> >     (search_type_for_mask): New.
>> >     (vect_recog_bool_pattern): Support cases when bool pattern
>> >     check fails.
>> >
>> >
>> >diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
>> >index 830801a..e3be3d1 100644
>> >--- a/gcc/tree-vect-patterns.c
>> >+++ b/gcc/tree-vect-patterns.c
>> >@@ -2962,6 +2962,11 @@ check_bool_pattern (tree var, loop_vec_info loop_vinfo, bb_vec_info bb_vinfo)
>> >       if (comp_vectype == NULL_TREE)
>> >         return false;
>> >
>> >+      mask_type = get_mask_type_for_scalar_type (TREE_TYPE (rhs1));
>> >+      if (mask_type
>> >+          && expand_vec_cmp_expr_p (comp_vectype, mask_type))
>> >+        return false;
>> >+
>> >       if (TREE_CODE (TREE_TYPE (rhs1)) != INTEGER_TYPE)
>> >         {
>> >           machine_mode mode = TYPE_MODE (TREE_TYPE (rhs1));
>> So we're essentially saying here that we've got another preferred method for
>> optimizing this case, right?
>>
>> Can you update the function comment for check_bool_pattern?  In particular
>> change the "if bool VAR can ..." to "can and should".
>>
>> I think that more clearly states the updated purpose of that code.
>>
>>
>>
>>
>> >@@ -3186,6 +3191,75 @@ adjust_bool_pattern (tree var, tree out_type, tree trueval,
>> >  }
>> >
>> >
>> >+/* Try to determine a proper type for converting bool VAR
>> >+   into an integer value.  The type is chosen so that
>> >+   conversion has the same number of elements as a mask
>> >+   producer.  */
>> >+
>> >+static tree
>> >+search_type_for_mask (tree var, loop_vec_info loop_vinfo, bb_vec_info bb_vinfo)
>> What is the return value here?  Presumably the type or NULL.
>>
>> So instead of "Try to determine a proper type" how about
>> "Return the proper type or NULL_TREE if no such type exists ..."?
>>
>> Please change the references to NULL to instead use NULL_TREE in that
>> function as well.  They're functionally equivalent, but the latter is
>> considered more correct these days.
>>
>>
>>
>> >+    {
>> >+      tree type = search_type_for_mask (var, loop_vinfo, bb_vinfo);
>> >+      tree cst0, cst1, cmp, tmp;
>> >+
>> >+      if (!type)
>> >+        return NULL;
>> >+
>> >+      /* We may directly use cond with narrowed type to avoid
>> >+         multiple cond exprs with following result packing and
>> >+         perform single cond with packed mask intead.  In case
>> s/intead/instead/
>>
>> With those changes above, this should be OK for the trunk.
>>
>> jeff
>>
>
> Thanks for review!  Here is an updated version with all mentioned issues fixed.

This results in bool patterns now never be used on x86_64 as we can do vec_cmp
for all subtargets.

Was this intended?

In theory it's nice to see a way to remove bool patterns (and
ifcvt_repair_bool_patterns!),
but doesn't this pessimize code on some subtargets?

Thanks,
Richard.

> Thanks,
> Ilya
> --
> 2015-09-12  Ilya Enkovich  <enkovich.gnu@gmail.com>
>
>         * tree-vect-patterns.c (check_bool_pattern): Check fails
>         if we can vectorize comparison directly.
>         (search_type_for_mask): New.
>         (vect_recog_bool_pattern): Support cases when bool pattern
>         check fails.
>
>
> diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
> index 3fe094c..516034d 100644
> --- a/gcc/tree-vect-patterns.c
> +++ b/gcc/tree-vect-patterns.c
> @@ -2879,7 +2879,9 @@ vect_recog_mixed_size_cond_pattern (vec<gimple *> *stmts, tree *type_in,
>
>
>  /* Helper function of vect_recog_bool_pattern.  Called recursively, return
> -   true if bool VAR can be optimized that way.  */
> +   true if bool VAR can and should be optimized that way.  Assume it shouldn't
> +   in case it's a result of a comparison which can be directly vectorized into
> +   a vector comparison.  */
>
>  static bool
>  check_bool_pattern (tree var, vec_info *vinfo)
> @@ -2928,7 +2930,7 @@ check_bool_pattern (tree var, vec_info *vinfo)
>      default:
>        if (TREE_CODE_CLASS (rhs_code) == tcc_comparison)
>         {
> -         tree vecitype, comp_vectype;
> +         tree vecitype, comp_vectype, mask_type;
>
>           /* If the comparison can throw, then is_gimple_condexpr will be
>              false and we can't make a COND_EXPR/VEC_COND_EXPR out of it.  */
> @@ -2939,6 +2941,11 @@ check_bool_pattern (tree var, vec_info *vinfo)
>           if (comp_vectype == NULL_TREE)
>             return false;
>
> +         mask_type = get_mask_type_for_scalar_type (TREE_TYPE (rhs1));
> +         if (mask_type
> +             && expand_vec_cmp_expr_p (comp_vectype, mask_type))
> +           return false;
> +
>           if (TREE_CODE (TREE_TYPE (rhs1)) != INTEGER_TYPE)
>             {
>               machine_mode mode = TYPE_MODE (TREE_TYPE (rhs1));
> @@ -3163,6 +3170,73 @@ adjust_bool_pattern (tree var, tree out_type, tree trueval,
>  }
>
>
> +/* Return the proper type for converting bool VAR into
> +   an integer value or NULL_TREE if no such type exists.
> +   The type is chosen so that converted value has the
> +   same number of elements as VAR's vector type.  */
> +
> +static tree
> +search_type_for_mask (tree var, vec_info *vinfo)
> +{
> +  gimple *def_stmt;
> +  enum vect_def_type dt;
> +  tree rhs1;
> +  enum tree_code rhs_code;
> +  tree res = NULL_TREE;
> +
> +  if (TREE_CODE (var) != SSA_NAME)
> +    return NULL_TREE;
> +
> +  if ((TYPE_PRECISION (TREE_TYPE (var)) != 1
> +       || !TYPE_UNSIGNED (TREE_TYPE (var)))
> +      && TREE_CODE (TREE_TYPE (var)) != BOOLEAN_TYPE)
> +    return NULL_TREE;
> +
> +  if (!vect_is_simple_use (var, vinfo, &def_stmt, &dt))
> +    return NULL_TREE;
> +
> +  if (dt != vect_internal_def)
> +    return NULL_TREE;
> +
> +  if (!is_gimple_assign (def_stmt))
> +    return NULL_TREE;
> +
> +  rhs_code = gimple_assign_rhs_code (def_stmt);
> +  rhs1 = gimple_assign_rhs1 (def_stmt);
> +
> +  switch (rhs_code)
> +    {
> +    case SSA_NAME:
> +    case BIT_NOT_EXPR:
> +    CASE_CONVERT:
> +      res = search_type_for_mask (rhs1, vinfo);
> +      break;
> +
> +    case BIT_AND_EXPR:
> +    case BIT_IOR_EXPR:
> +    case BIT_XOR_EXPR:
> +      if (!(res = search_type_for_mask (rhs1, vinfo)))
> +       res = search_type_for_mask (gimple_assign_rhs2 (def_stmt), vinfo);
> +      break;
> +
> +    default:
> +      if (TREE_CODE_CLASS (rhs_code) == tcc_comparison)
> +       {
> +         if (TREE_CODE (TREE_TYPE (rhs1)) != INTEGER_TYPE
> +             || !TYPE_UNSIGNED (TREE_TYPE (rhs1)))
> +           {
> +             machine_mode mode = TYPE_MODE (TREE_TYPE (rhs1));
> +             res = build_nonstandard_integer_type (GET_MODE_BITSIZE (mode), 1);
> +           }
> +         else
> +           res = TREE_TYPE (rhs1);
> +       }
> +    }
> +
> +  return res;
> +}
> +
> +
>  /* Function vect_recog_bool_pattern
>
>     Try to find pattern like following:
> @@ -3220,6 +3294,7 @@ vect_recog_bool_pattern (vec<gimple *> *stmts, tree *type_in,
>    enum tree_code rhs_code;
>    tree var, lhs, rhs, vectype;
>    stmt_vec_info stmt_vinfo = vinfo_for_stmt (last_stmt);
> +  stmt_vec_info new_stmt_info;
>    vec_info *vinfo = stmt_vinfo->vinfo;
>    gimple *pattern_stmt;
>
> @@ -3244,16 +3319,52 @@ vect_recog_bool_pattern (vec<gimple *> *stmts, tree *type_in,
>        if (vectype == NULL_TREE)
>         return NULL;
>
> -      if (!check_bool_pattern (var, vinfo))
> -       return NULL;
> -
> -      rhs = adjust_bool_pattern (var, TREE_TYPE (lhs), NULL_TREE, stmts);
> -      lhs = vect_recog_temp_ssa_var (TREE_TYPE (lhs), NULL);
> -      if (useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
> -       pattern_stmt = gimple_build_assign (lhs, SSA_NAME, rhs);
> +      if (check_bool_pattern (var, vinfo))
> +       {
> +         rhs = adjust_bool_pattern (var, TREE_TYPE (lhs), NULL_TREE, stmts);
> +         lhs = vect_recog_temp_ssa_var (TREE_TYPE (lhs), NULL);
> +         if (useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
> +           pattern_stmt = gimple_build_assign (lhs, SSA_NAME, rhs);
> +         else
> +           pattern_stmt
> +             = gimple_build_assign (lhs, NOP_EXPR, rhs);
> +       }
>        else
> -       pattern_stmt
> -         = gimple_build_assign (lhs, NOP_EXPR, rhs);
> +       {
> +         tree type = search_type_for_mask (var, vinfo);
> +         tree cst0, cst1, cmp, tmp;
> +
> +         if (!type)
> +           return NULL;
> +
> +         /* We may directly use cond with narrowed type to avoid
> +            multiple cond exprs with following result packing and
> +            perform single cond with packed mask instead.  In case
> +            of widening we better make cond first and then extract
> +            results.  */
> +         if (TYPE_MODE (type) == TYPE_MODE (TREE_TYPE (lhs)))
> +           type = TREE_TYPE (lhs);
> +
> +         cst0 = build_int_cst (type, 0);
> +         cst1 = build_int_cst (type, 1);
> +         tmp = vect_recog_temp_ssa_var (type, NULL);
> +         cmp = build2 (NE_EXPR, boolean_type_node,
> +                       var, build_int_cst (TREE_TYPE (var), 0));
> +         pattern_stmt = gimple_build_assign (tmp, COND_EXPR, cmp, cst1, cst0);
> +
> +         if (!useless_type_conversion_p (type, TREE_TYPE (lhs)))
> +           {
> +             tree new_vectype = get_vectype_for_scalar_type (type);
> +             new_stmt_info = new_stmt_vec_info (pattern_stmt, vinfo);
> +             set_vinfo_for_stmt (pattern_stmt, new_stmt_info);
> +             STMT_VINFO_VECTYPE (new_stmt_info) = new_vectype;
> +             new_pattern_def_seq (stmt_vinfo, pattern_stmt);
> +
> +             lhs = vect_recog_temp_ssa_var (TREE_TYPE (lhs), NULL);
> +             pattern_stmt = gimple_build_assign (lhs, CONVERT_EXPR, tmp);
> +           }
> +       }
> +
>        *type_out = vectype;
>        *type_in = vectype;
>        stmts->safe_push (last_stmt);
> @@ -3282,15 +3393,19 @@ vect_recog_bool_pattern (vec<gimple *> *stmts, tree *type_in,
>        if (get_vectype_for_scalar_type (type) == NULL_TREE)
>         return NULL;
>
> -      if (!check_bool_pattern (var, vinfo))
> -       return NULL;
> +      if (check_bool_pattern (var, vinfo))
> +       {
> +         rhs = adjust_bool_pattern (var, type, NULL_TREE, stmts);
> +         rhs = build2 (NE_EXPR, boolean_type_node,
> +                       rhs, build_int_cst (type, 0));
> +       }
> +      else
> +       rhs = build2 (NE_EXPR, boolean_type_node,
> +                     var, build_int_cst (TREE_TYPE (var), 0)),
>
> -      rhs = adjust_bool_pattern (var, type, NULL_TREE, stmts);
>        lhs = vect_recog_temp_ssa_var (TREE_TYPE (lhs), NULL);
>        pattern_stmt
> -         = gimple_build_assign (lhs, COND_EXPR,
> -                                build2 (NE_EXPR, boolean_type_node,
> -                                        rhs, build_int_cst (type, 0)),
> +         = gimple_build_assign (lhs, COND_EXPR, rhs,
>                                  gimple_assign_rhs2 (last_stmt),
>                                  gimple_assign_rhs3 (last_stmt));
>        *type_out = vectype;
> @@ -3310,16 +3425,43 @@ vect_recog_bool_pattern (vec<gimple *> *stmts, tree *type_in,
>        gcc_assert (vectype != NULL_TREE);
>        if (!VECTOR_MODE_P (TYPE_MODE (vectype)))
>         return NULL;
> -      if (!check_bool_pattern (var, vinfo))
> -       return NULL;
>
> -      rhs = adjust_bool_pattern (var, TREE_TYPE (vectype), NULL_TREE, stmts);
> +      if (check_bool_pattern (var, vinfo))
> +       rhs = adjust_bool_pattern (var, TREE_TYPE (vectype),
> +                                  NULL_TREE, stmts);
> +      else
> +       {
> +         tree type = search_type_for_mask (var, vinfo);
> +         tree cst0, cst1, cmp, new_vectype;
> +
> +         if (!type)
> +           return NULL;
> +
> +         if (TYPE_MODE (type) == TYPE_MODE (TREE_TYPE (vectype)))
> +           type = TREE_TYPE (vectype);
> +
> +         cst0 = build_int_cst (type, 0);
> +         cst1 = build_int_cst (type, 1);
> +         new_vectype = get_vectype_for_scalar_type (type);
> +
> +         rhs = vect_recog_temp_ssa_var (type, NULL);
> +         cmp = build2 (NE_EXPR, boolean_type_node,
> +                       var, build_int_cst (TREE_TYPE (var), 0));
> +         pattern_stmt = gimple_build_assign (rhs, COND_EXPR,
> +                                             cmp, cst1, cst0);
> +
> +         pattern_stmt_info = new_stmt_vec_info (pattern_stmt, vinfo);
> +         set_vinfo_for_stmt (pattern_stmt, pattern_stmt_info);
> +         STMT_VINFO_VECTYPE (pattern_stmt_info) = new_vectype;
> +         append_pattern_def_seq (stmt_vinfo, pattern_stmt);
> +       }
> +
>        lhs = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (vectype), lhs);
>        if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
>         {
>           tree rhs2 = vect_recog_temp_ssa_var (TREE_TYPE (lhs), NULL);
>           gimple *cast_stmt = gimple_build_assign (rhs2, NOP_EXPR, rhs);
> -         new_pattern_def_seq (stmt_vinfo, cast_stmt);
> +         append_pattern_def_seq (stmt_vinfo, cast_stmt);
>           rhs = rhs2;
>         }
>        pattern_stmt = gimple_build_assign (lhs, SSA_NAME, rhs);
Ilya Enkovich May 25, 2016, 12:02 p.m. UTC | #3
2016-05-25 14:37 GMT+03:00 Richard Biener <richard.guenther@gmail.com>:
> On Thu, Oct 22, 2015 at 6:04 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> On 21 Oct 11:45, Jeff Law wrote:
>>> On 10/08/2015 09:15 AM, Ilya Enkovich wrote:
>>> >Hi,
>>> >
>>> >This patch disables transformation of boolean computations into integer ones in case target supports vector comparison.  Pattern still applies to transform resulting boolean value into integer or avoid COND_EXPR with SSA_NAME as condition.
>>> >
>>> >Thanks,
>>> >Ilya
>>> >--
>>> >2015-10-08  Ilya Enkovich  <enkovich.gnu@gmail.com>
>>> >
>>> >     * tree-vect-patterns.c (check_bool_pattern): Check fails
>>> >     if we can vectorize comparison directly.
>>> >     (search_type_for_mask): New.
>>> >     (vect_recog_bool_pattern): Support cases when bool pattern
>>> >     check fails.
>>> >
>>> >
>>> >diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
>>> >index 830801a..e3be3d1 100644
>>> >--- a/gcc/tree-vect-patterns.c
>>> >+++ b/gcc/tree-vect-patterns.c
>>> >@@ -2962,6 +2962,11 @@ check_bool_pattern (tree var, loop_vec_info loop_vinfo, bb_vec_info bb_vinfo)
>>> >       if (comp_vectype == NULL_TREE)
>>> >         return false;
>>> >
>>> >+      mask_type = get_mask_type_for_scalar_type (TREE_TYPE (rhs1));
>>> >+      if (mask_type
>>> >+          && expand_vec_cmp_expr_p (comp_vectype, mask_type))
>>> >+        return false;
>>> >+
>>> >       if (TREE_CODE (TREE_TYPE (rhs1)) != INTEGER_TYPE)
>>> >         {
>>> >           machine_mode mode = TYPE_MODE (TREE_TYPE (rhs1));
>>> So we're essentially saying here that we've got another preferred method for
>>> optimizing this case, right?
>>>
>>> Can you update the function comment for check_bool_pattern?  In particular
>>> change the "if bool VAR can ..." to "can and should".
>>>
>>> I think that more clearly states the updated purpose of that code.
>>>
>>>
>>>
>>>
>>> >@@ -3186,6 +3191,75 @@ adjust_bool_pattern (tree var, tree out_type, tree trueval,
>>> >  }
>>> >
>>> >
>>> >+/* Try to determine a proper type for converting bool VAR
>>> >+   into an integer value.  The type is chosen so that
>>> >+   conversion has the same number of elements as a mask
>>> >+   producer.  */
>>> >+
>>> >+static tree
>>> >+search_type_for_mask (tree var, loop_vec_info loop_vinfo, bb_vec_info bb_vinfo)
>>> What is the return value here?  Presumably the type or NULL.
>>>
>>> So instead of "Try to determine a proper type" how about
>>> "Return the proper type or NULL_TREE if no such type exists ..."?
>>>
>>> Please change the references to NULL to instead use NULL_TREE in that
>>> function as well.  They're functionally equivalent, but the latter is
>>> considered more correct these days.
>>>
>>>
>>>
>>> >+    {
>>> >+      tree type = search_type_for_mask (var, loop_vinfo, bb_vinfo);
>>> >+      tree cst0, cst1, cmp, tmp;
>>> >+
>>> >+      if (!type)
>>> >+        return NULL;
>>> >+
>>> >+      /* We may directly use cond with narrowed type to avoid
>>> >+         multiple cond exprs with following result packing and
>>> >+         perform single cond with packed mask intead.  In case
>>> s/intead/instead/
>>>
>>> With those changes above, this should be OK for the trunk.
>>>
>>> jeff
>>>
>>
>> Thanks for review!  Here is an updated version with all mentioned issues fixed.
>
> This results in bool patterns now never be used on x86_64 as we can do vec_cmp
> for all subtargets.
>
> Was this intended?

This was intentional and I saw better code generated for SSE and AVX targets due
to reuse of comparison result.

>
> In theory it's nice to see a way to remove bool patterns (and
> ifcvt_repair_bool_patterns!),
> but doesn't this pessimize code on some subtargets?

I don't think bool patterns are not used at all now for x86_64.  IIRC
bool patterns
not involving comparisons still should apply.  E.g. simple bool to int
conversion
goes through bool patterns.  Bool stores (and loads?) should also
trigger bool patterns.

Obviously removing bool patterns would require vec_cmp support on all
targets which
support vec_cond now.  I'm pretty sure we should be able to get the
same code quality
using boolean vectors instead of integer ones.

I think the first step in bool patterns removal should be making
check_bool_pattern
always return false and then look at the rest we have in
vect_recog_bool_pattern.

Thanks,
Ilya

>
> Thanks,
> Richard.
>
>> Thanks,
>> Ilya
diff mbox

Patch

diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
index 3fe094c..516034d 100644
--- a/gcc/tree-vect-patterns.c
+++ b/gcc/tree-vect-patterns.c
@@ -2879,7 +2879,9 @@  vect_recog_mixed_size_cond_pattern (vec<gimple *> *stmts, tree *type_in,
 
 
 /* Helper function of vect_recog_bool_pattern.  Called recursively, return
-   true if bool VAR can be optimized that way.  */
+   true if bool VAR can and should be optimized that way.  Assume it shouldn't
+   in case it's a result of a comparison which can be directly vectorized into
+   a vector comparison.  */
 
 static bool
 check_bool_pattern (tree var, vec_info *vinfo)
@@ -2928,7 +2930,7 @@  check_bool_pattern (tree var, vec_info *vinfo)
     default:
       if (TREE_CODE_CLASS (rhs_code) == tcc_comparison)
 	{
-	  tree vecitype, comp_vectype;
+	  tree vecitype, comp_vectype, mask_type;
 
 	  /* If the comparison can throw, then is_gimple_condexpr will be
 	     false and we can't make a COND_EXPR/VEC_COND_EXPR out of it.  */
@@ -2939,6 +2941,11 @@  check_bool_pattern (tree var, vec_info *vinfo)
 	  if (comp_vectype == NULL_TREE)
 	    return false;
 
+	  mask_type = get_mask_type_for_scalar_type (TREE_TYPE (rhs1));
+	  if (mask_type
+	      && expand_vec_cmp_expr_p (comp_vectype, mask_type))
+	    return false;
+
 	  if (TREE_CODE (TREE_TYPE (rhs1)) != INTEGER_TYPE)
 	    {
 	      machine_mode mode = TYPE_MODE (TREE_TYPE (rhs1));
@@ -3163,6 +3170,73 @@  adjust_bool_pattern (tree var, tree out_type, tree trueval,
 }
 
 
+/* Return the proper type for converting bool VAR into
+   an integer value or NULL_TREE if no such type exists.
+   The type is chosen so that converted value has the
+   same number of elements as VAR's vector type.  */
+
+static tree
+search_type_for_mask (tree var, vec_info *vinfo)
+{
+  gimple *def_stmt;
+  enum vect_def_type dt;
+  tree rhs1;
+  enum tree_code rhs_code;
+  tree res = NULL_TREE;
+
+  if (TREE_CODE (var) != SSA_NAME)
+    return NULL_TREE;
+
+  if ((TYPE_PRECISION (TREE_TYPE (var)) != 1
+       || !TYPE_UNSIGNED (TREE_TYPE (var)))
+      && TREE_CODE (TREE_TYPE (var)) != BOOLEAN_TYPE)
+    return NULL_TREE;
+
+  if (!vect_is_simple_use (var, vinfo, &def_stmt, &dt))
+    return NULL_TREE;
+
+  if (dt != vect_internal_def)
+    return NULL_TREE;
+
+  if (!is_gimple_assign (def_stmt))
+    return NULL_TREE;
+
+  rhs_code = gimple_assign_rhs_code (def_stmt);
+  rhs1 = gimple_assign_rhs1 (def_stmt);
+
+  switch (rhs_code)
+    {
+    case SSA_NAME:
+    case BIT_NOT_EXPR:
+    CASE_CONVERT:
+      res = search_type_for_mask (rhs1, vinfo);
+      break;
+
+    case BIT_AND_EXPR:
+    case BIT_IOR_EXPR:
+    case BIT_XOR_EXPR:
+      if (!(res = search_type_for_mask (rhs1, vinfo)))
+	res = search_type_for_mask (gimple_assign_rhs2 (def_stmt), vinfo);
+      break;
+
+    default:
+      if (TREE_CODE_CLASS (rhs_code) == tcc_comparison)
+	{
+	  if (TREE_CODE (TREE_TYPE (rhs1)) != INTEGER_TYPE
+	      || !TYPE_UNSIGNED (TREE_TYPE (rhs1)))
+	    {
+	      machine_mode mode = TYPE_MODE (TREE_TYPE (rhs1));
+	      res = build_nonstandard_integer_type (GET_MODE_BITSIZE (mode), 1);
+	    }
+	  else
+	    res = TREE_TYPE (rhs1);
+	}
+    }
+
+  return res;
+}
+
+
 /* Function vect_recog_bool_pattern
 
    Try to find pattern like following:
@@ -3220,6 +3294,7 @@  vect_recog_bool_pattern (vec<gimple *> *stmts, tree *type_in,
   enum tree_code rhs_code;
   tree var, lhs, rhs, vectype;
   stmt_vec_info stmt_vinfo = vinfo_for_stmt (last_stmt);
+  stmt_vec_info new_stmt_info;
   vec_info *vinfo = stmt_vinfo->vinfo;
   gimple *pattern_stmt;
 
@@ -3244,16 +3319,52 @@  vect_recog_bool_pattern (vec<gimple *> *stmts, tree *type_in,
       if (vectype == NULL_TREE)
 	return NULL;
 
-      if (!check_bool_pattern (var, vinfo))
-	return NULL;
-
-      rhs = adjust_bool_pattern (var, TREE_TYPE (lhs), NULL_TREE, stmts);
-      lhs = vect_recog_temp_ssa_var (TREE_TYPE (lhs), NULL);
-      if (useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
-	pattern_stmt = gimple_build_assign (lhs, SSA_NAME, rhs);
+      if (check_bool_pattern (var, vinfo))
+	{
+	  rhs = adjust_bool_pattern (var, TREE_TYPE (lhs), NULL_TREE, stmts);
+	  lhs = vect_recog_temp_ssa_var (TREE_TYPE (lhs), NULL);
+	  if (useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
+	    pattern_stmt = gimple_build_assign (lhs, SSA_NAME, rhs);
+	  else
+	    pattern_stmt
+	      = gimple_build_assign (lhs, NOP_EXPR, rhs);
+	}
       else
-	pattern_stmt
-	  = gimple_build_assign (lhs, NOP_EXPR, rhs);
+	{
+	  tree type = search_type_for_mask (var, vinfo);
+	  tree cst0, cst1, cmp, tmp;
+
+	  if (!type)
+	    return NULL;
+
+	  /* We may directly use cond with narrowed type to avoid
+	     multiple cond exprs with following result packing and
+	     perform single cond with packed mask instead.  In case
+	     of widening we better make cond first and then extract
+	     results.  */
+	  if (TYPE_MODE (type) == TYPE_MODE (TREE_TYPE (lhs)))
+	    type = TREE_TYPE (lhs);
+
+	  cst0 = build_int_cst (type, 0);
+	  cst1 = build_int_cst (type, 1);
+	  tmp = vect_recog_temp_ssa_var (type, NULL);
+	  cmp = build2 (NE_EXPR, boolean_type_node,
+			var, build_int_cst (TREE_TYPE (var), 0));
+	  pattern_stmt = gimple_build_assign (tmp, COND_EXPR, cmp, cst1, cst0);
+
+	  if (!useless_type_conversion_p (type, TREE_TYPE (lhs)))
+	    {
+	      tree new_vectype = get_vectype_for_scalar_type (type);
+	      new_stmt_info = new_stmt_vec_info (pattern_stmt, vinfo);
+	      set_vinfo_for_stmt (pattern_stmt, new_stmt_info);
+	      STMT_VINFO_VECTYPE (new_stmt_info) = new_vectype;
+	      new_pattern_def_seq (stmt_vinfo, pattern_stmt);
+
+	      lhs = vect_recog_temp_ssa_var (TREE_TYPE (lhs), NULL);
+	      pattern_stmt = gimple_build_assign (lhs, CONVERT_EXPR, tmp);
+	    }
+	}
+
       *type_out = vectype;
       *type_in = vectype;
       stmts->safe_push (last_stmt);
@@ -3282,15 +3393,19 @@  vect_recog_bool_pattern (vec<gimple *> *stmts, tree *type_in,
       if (get_vectype_for_scalar_type (type) == NULL_TREE)
 	return NULL;
 
-      if (!check_bool_pattern (var, vinfo))
-	return NULL;
+      if (check_bool_pattern (var, vinfo))
+	{
+	  rhs = adjust_bool_pattern (var, type, NULL_TREE, stmts);
+	  rhs = build2 (NE_EXPR, boolean_type_node,
+			rhs, build_int_cst (type, 0));
+	}
+      else
+	rhs = build2 (NE_EXPR, boolean_type_node,
+		      var, build_int_cst (TREE_TYPE (var), 0)),
 
-      rhs = adjust_bool_pattern (var, type, NULL_TREE, stmts);
       lhs = vect_recog_temp_ssa_var (TREE_TYPE (lhs), NULL);
       pattern_stmt 
-	  = gimple_build_assign (lhs, COND_EXPR,
-				 build2 (NE_EXPR, boolean_type_node,
-					 rhs, build_int_cst (type, 0)),
+	  = gimple_build_assign (lhs, COND_EXPR, rhs,
 				 gimple_assign_rhs2 (last_stmt),
 				 gimple_assign_rhs3 (last_stmt));
       *type_out = vectype;
@@ -3310,16 +3425,43 @@  vect_recog_bool_pattern (vec<gimple *> *stmts, tree *type_in,
       gcc_assert (vectype != NULL_TREE);
       if (!VECTOR_MODE_P (TYPE_MODE (vectype)))
 	return NULL;
-      if (!check_bool_pattern (var, vinfo))
-	return NULL;
 
-      rhs = adjust_bool_pattern (var, TREE_TYPE (vectype), NULL_TREE, stmts);
+      if (check_bool_pattern (var, vinfo))
+	rhs = adjust_bool_pattern (var, TREE_TYPE (vectype),
+				   NULL_TREE, stmts);
+      else
+	{
+	  tree type = search_type_for_mask (var, vinfo);
+	  tree cst0, cst1, cmp, new_vectype;
+
+	  if (!type)
+	    return NULL;
+
+	  if (TYPE_MODE (type) == TYPE_MODE (TREE_TYPE (vectype)))
+	    type = TREE_TYPE (vectype);
+
+	  cst0 = build_int_cst (type, 0);
+	  cst1 = build_int_cst (type, 1);
+	  new_vectype = get_vectype_for_scalar_type (type);
+
+	  rhs = vect_recog_temp_ssa_var (type, NULL);
+	  cmp = build2 (NE_EXPR, boolean_type_node,
+			var, build_int_cst (TREE_TYPE (var), 0));
+	  pattern_stmt = gimple_build_assign (rhs, COND_EXPR,
+					      cmp, cst1, cst0);
+
+	  pattern_stmt_info = new_stmt_vec_info (pattern_stmt, vinfo);
+	  set_vinfo_for_stmt (pattern_stmt, pattern_stmt_info);
+	  STMT_VINFO_VECTYPE (pattern_stmt_info) = new_vectype;
+	  append_pattern_def_seq (stmt_vinfo, pattern_stmt);
+	}
+
       lhs = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (vectype), lhs);
       if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
 	{
 	  tree rhs2 = vect_recog_temp_ssa_var (TREE_TYPE (lhs), NULL);
 	  gimple *cast_stmt = gimple_build_assign (rhs2, NOP_EXPR, rhs);
-	  new_pattern_def_seq (stmt_vinfo, cast_stmt);
+	  append_pattern_def_seq (stmt_vinfo, cast_stmt);
 	  rhs = rhs2;
 	}
       pattern_stmt = gimple_build_assign (lhs, SSA_NAME, rhs);