2016-05-25 14:37 GMT+03:00 Richard Biener <richard.guent...@gmail.com>:
> 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?
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
