[vec-cmp, patch 4/6] Support vector mask invariants
Richard Biener
richard.guenther@gmail.com
Mon Oct 26 15:25:00 GMT 2015
On Wed, Oct 14, 2015 at 6:13 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> On 14 Oct 13:50, Ilya Enkovich wrote:
>> 2015-10-14 11:49 GMT+03:00 Richard Biener <richard.guenther@gmail.com>:
>> > On Tue, Oct 13, 2015 at 4:52 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> >> I don't understand what you mean. vect_get_vec_def_for_operand has two
>> >> changes made.
>> >> 1. For boolean invariants use build_same_sized_truth_vector_type
>> >> instead of get_vectype_for_scalar_type in case statement produces a
>> >> boolean vector. This covers cases when we use invariants in
>> >> comparison, AND, IOR, XOR.
>> >
>> > Yes, I understand we need this special-casing to differentiate between
>> > the vector type
>> > used for boolean-typed loads/stores and the type for boolean typed constants.
>> > What happens if we mix them btw, like with
>> >
>> > _Bool b = bools[i];
>> > _Bool c = b || d;
>> > ...
>> >
>> > ?
>>
>> Here both statements should get vector of char as a vectype and we
>> never go VECTOR_BOOLEAN_TYPE_P way for them
>>
>> >
>> >> 2. COND_EXPR is an exception because it has built-in boolean vector
>> >> result not reflected in its vecinfo. Thus I added additional operand
>> >> for vect_get_vec_def_for_operand to directly specify vectype for
>> >> vector definition in case it is a loop invariant.
>> >> So what do you propose to do with these changes?
>> >
>> > This is the change I don't like and don't see why we need it. It works today
>> > and the comparison operands should be of appropriate type already?
>>
>> Today it works because we always create vector of integer constant.
>> With boolean vectors it may be either integer vector or boolean vector
>> depending on context. Consider:
>>
>> _Bool _1;
>> int _2;
>>
>> _2 = _1 != 0 ? 0 : 1
>>
>> We have two zero constants here requiring different vectypes.
>>
>> Ilya
>>
>> >
>> > Richard.
>> >
>> >> Thanks,
>> >> Ilya
>
> Here is an updated patch version.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2015-10-14 Ilya Enkovich <enkovich.gnu@gmail.com>
>
> * expr.c (const_vector_mask_from_tree): New.
> (const_vector_from_tree): Use const_vector_mask_from_tree
> for boolean vectors.
> * tree-vect-stmts.c (vect_init_vector): Support boolean vector
> invariants.
> (vect_get_vec_def_for_operand): Add VECTYPE arg.
> (vectorizable_condition): Directly provide vectype for invariants
> used in comparison.
> * tree-vectorizer.h (vect_get_vec_def_for_operand): Add VECTYPE
> arg.
>
>
> diff --git a/gcc/expr.c b/gcc/expr.c
> index b5ff598..ab25d1a 100644
> --- a/gcc/expr.c
> +++ b/gcc/expr.c
> @@ -11344,6 +11344,40 @@ try_tablejump (tree index_type, tree index_expr, tree minval, tree range,
> return 1;
> }
>
> +/* Return a CONST_VECTOR rtx representing vector mask for
> + a VECTOR_CST of booleans. */
> +static rtx
> +const_vector_mask_from_tree (tree exp)
> +{
> + rtvec v;
> + unsigned i;
> + int units;
> + tree elt;
> + machine_mode inner, mode;
> +
> + mode = TYPE_MODE (TREE_TYPE (exp));
> + units = GET_MODE_NUNITS (mode);
> + inner = GET_MODE_INNER (mode);
> +
> + v = rtvec_alloc (units);
> +
> + for (i = 0; i < VECTOR_CST_NELTS (exp); ++i)
> + {
> + elt = VECTOR_CST_ELT (exp, i);
> +
> + gcc_assert (TREE_CODE (elt) == INTEGER_CST);
> + if (integer_zerop (elt))
> + RTVEC_ELT (v, i) = CONST0_RTX (inner);
> + else if (integer_onep (elt)
> + || integer_minus_onep (elt))
> + RTVEC_ELT (v, i) = CONSTM1_RTX (inner);
> + else
> + gcc_unreachable ();
> + }
> +
> + return gen_rtx_CONST_VECTOR (mode, v);
> +}
> +
> /* Return a CONST_VECTOR rtx for a VECTOR_CST tree. */
> static rtx
> const_vector_from_tree (tree exp)
> @@ -11359,6 +11393,9 @@ const_vector_from_tree (tree exp)
> if (initializer_zerop (exp))
> return CONST0_RTX (mode);
>
> + if (VECTOR_BOOLEAN_TYPE_P (TREE_TYPE (exp)))
> + return const_vector_mask_from_tree (exp);
> +
> units = GET_MODE_NUNITS (mode);
> inner = GET_MODE_INNER (mode);
>
> diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
> index 6a52895..01168ae 100644
> --- a/gcc/tree-vect-stmts.c
> +++ b/gcc/tree-vect-stmts.c
> @@ -1308,7 +1308,22 @@ vect_init_vector (gimple *stmt, tree val, tree type, gimple_stmt_iterator *gsi)
> if (!types_compatible_p (TREE_TYPE (type), TREE_TYPE (val)))
> {
> if (CONSTANT_CLASS_P (val))
> - val = fold_unary (VIEW_CONVERT_EXPR, TREE_TYPE (type), val);
> + {
> + /* Can't use VIEW_CONVERT_EXPR for booleans because
> + of possibly different sizes of scalar value and
> + vector element. */
> + if (VECTOR_BOOLEAN_TYPE_P (type))
> + {
> + if (integer_zerop (val))
> + val = build_int_cst (TREE_TYPE (type), 0);
> + else if (integer_onep (val))
> + val = build_int_cst (TREE_TYPE (type), 1);
> + else
> + gcc_unreachable ();
> + }
> + else
> + val = fold_unary (VIEW_CONVERT_EXPR, TREE_TYPE (type), val);
I think the existing code is fine with using fold_convert () here
which should also work
for the boolean types. So does just
val = fold_convert (TREE_TYPE (type), val);
work?
> + }
> else
> {
> new_temp = make_ssa_name (TREE_TYPE (type));
> @@ -1339,16 +1354,19 @@ vect_init_vector (gimple *stmt, tree val, tree type, gimple_stmt_iterator *gsi)
> STMT_VINFO_VEC_STMT of the defining stmt holds the relevant def.
>
> In case OP is an invariant or constant, a new stmt that creates a vector def
> - needs to be introduced. */
> + needs to be introduced. VECTYPE may be used to specify a required type for
> + vector invariant. */
>
> tree
> -vect_get_vec_def_for_operand (tree op, gimple *stmt, tree *scalar_def)
> +vect_get_vec_def_for_operand (tree op, gimple *stmt, tree *scalar_def,
> + tree vectype)
> {
> tree vec_oprnd;
> gimple *vec_stmt;
> gimple *def_stmt;
> stmt_vec_info def_stmt_info = NULL;
> stmt_vec_info stmt_vinfo = vinfo_for_stmt (stmt);
> + tree stmt_vectype = STMT_VINFO_VECTYPE (stmt_vinfo);
> unsigned int nunits;
> loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_vinfo);
> tree def;
> @@ -1392,7 +1410,14 @@ vect_get_vec_def_for_operand (tree op, gimple *stmt, tree *scalar_def)
> /* Case 1: operand is a constant. */
> case vect_constant_def:
> {
> - vector_type = get_vectype_for_scalar_type (TREE_TYPE (op));
> + if (vectype)
> + vector_type = vectype;
> + else if (TREE_CODE (TREE_TYPE (op)) == BOOLEAN_TYPE
> + && VECTOR_BOOLEAN_TYPE_P (stmt_vectype))
> + vector_type = build_same_sized_truth_vector_type (stmt_vectype);
> + else
> + vector_type = get_vectype_for_scalar_type (TREE_TYPE (op));
> +
> gcc_assert (vector_type);
> nunits = TYPE_VECTOR_SUBPARTS (vector_type);
>
> @@ -1410,7 +1435,13 @@ vect_get_vec_def_for_operand (tree op, gimple *stmt, tree *scalar_def)
> /* Case 2: operand is defined outside the loop - loop invariant. */
> case vect_external_def:
> {
> - vector_type = get_vectype_for_scalar_type (TREE_TYPE (def));
> + if (vectype)
> + vector_type = vectype;
> + else if (TREE_CODE (TREE_TYPE (op)) == BOOLEAN_TYPE
> + && VECTOR_BOOLEAN_TYPE_P (stmt_vectype))
> + vector_type = build_same_sized_truth_vector_type (stmt_vectype);
> + else
> + vector_type = get_vectype_for_scalar_type (TREE_TYPE (def));
> gcc_assert (vector_type);
>
> if (scalar_def)
> @@ -7428,13 +7459,13 @@ vectorizable_condition (gimple *stmt, gimple_stmt_iterator *gsi,
> gimple *gtemp;
> vec_cond_lhs =
> vect_get_vec_def_for_operand (TREE_OPERAND (cond_expr, 0),
> - stmt, NULL);
> + stmt, NULL, comp_vectype);
> vect_is_simple_use (TREE_OPERAND (cond_expr, 0), stmt,
> loop_vinfo, >emp, &def, &dts[0]);
>
> vec_cond_rhs =
> vect_get_vec_def_for_operand (TREE_OPERAND (cond_expr, 1),
> - stmt, NULL);
> + stmt, NULL, comp_vectype);
> vect_is_simple_use (TREE_OPERAND (cond_expr, 1), stmt,
> loop_vinfo, >emp, &def, &dts[1]);
I still don't like this very much but I guess without some major
refactoring of all
the functions there isn't a better way to do it for now.
Thus, ok with trying the change suggested above.
Thanks,
Richard.
> if (reduc_index == 1)
> diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
> index f8d1e97..8eca7be 100644
> --- a/gcc/tree-vectorizer.h
> +++ b/gcc/tree-vectorizer.h
> @@ -994,7 +994,7 @@ extern unsigned record_stmt_cost (stmt_vector_for_cost *, int,
> extern void vect_finish_stmt_generation (gimple *, gimple *,
> gimple_stmt_iterator *);
> extern bool vect_mark_stmts_to_be_vectorized (loop_vec_info);
> -extern tree vect_get_vec_def_for_operand (tree, gimple *, tree *);
> +extern tree vect_get_vec_def_for_operand (tree, gimple *, tree *, tree = NULL);
> extern tree vect_init_vector (gimple *, tree, tree,
> gimple_stmt_iterator *);
> extern tree vect_get_vec_def_for_stmt_copy (enum vect_def_type, tree);
More information about the Gcc-patches
mailing list