On Thu, Oct 22, 2015 at 6:04 PM, Ilya Enkovich <enkovich....@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....@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....@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);
