On Mon, Aug 22, 2011 at 2:05 PM, Artem Shinkarov
<artyom.shinkar...@gmail.com> wrote:
> On Mon, Aug 22, 2011 at 12:25 PM, Richard Guenther
> <richard.guent...@gmail.com> wrote:
>> On Mon, Aug 22, 2011 at 12:53 AM, Artem Shinkarov
>> <artyom.shinkar...@gmail.com> wrote:
>>> Richard
>>>
>>> I formalized an approach a little-bit, now it works without target
>>> hooks, but some polishing is still required. I want you to comment on
>>> the several important approaches that I use in the patch.
>>>
>>> So how does it work.
>>> 1) All the vector comparisons at the level of type-checker are
>>> introduced using VEC_COND_EXPR with constant selection operands being
>>> {-1} and {0}. For example v0 > v1 is transformed into VEC_COND_EXPR<v0
>>>> v1, {-1}, {0}>.
>>>
>>> 2) When optabs expand VEC_COND_EXPR, two cases are considered:
>>> 2.a) first operand of VEC_COND_EXPR is comparison, in that case nothing
>>> changes.
>>> 2.b) first operand is something else, in that case, we specially mark
>>> this case, recognize it in the backend, and do not create a
>>> comparison, but use the mask as it was a result of some comparison.
>>>
>>> 3) In order to make sure that mask in VEC_COND_EXPR<mask, v0, v1> is a
>>> vector comparison we use is_vector_comparison function, if it returns
>>> false, then we replace mask with mask != {0}.
>>>
>>> So we end-up with the following functionality:
>>> VEC_COND_EXPR<mask, v0,v1> -- if we know that mask is a result of
>>> comparison of two vectors, we leave it as it is, otherwise change with
>>> mask != {0}.
>>>
>>> Plain vector comparison a <op> b is represented with VEC_COND_EXPR,
>>> which correctly expands, without creating useless masking.
>>>
>>>
>>> Basically for me there are two questions:
>>> 1) Can we perform information passing in optabs in a nicer way?
>>> 2) How is_vector_comparison could be improved? I have several ideas,
>>> like checking if constant vector all consists of 0 and -1, and so on.
>>> But first is it conceptually fine.
>>>
>>> P.S. I tired to put the functionality of is_vector_comparison in
>>> tree-ssa-forwprop, but the thing is that it is called only with -On,
>>> which I find inappropriate, and the functionality gets more
>>> complicated.
>>
>> Why is it inappropriate to not optimize it at -O0? If the user
>> separates comparison and ?: expression it's his own fault.
>
> Well, because all the information is there, and I perfectly envision
> the case when user expressed comparison separately, just to avoid code
> duplication.
>
> Like:
> mask = a > b;
> res1 = mask ? v0 : v1;
> res2 = mask ? v2 : v3;
>
> Which in this case would be different from
> res1 = a > b ? v0 : v1;
> res2 = a > b ? v2 : v3;
>
>> Btw, the new hook is still in the patch.
>>
>> I would simply always create != 0 if it isn't and let optimizers
>> (tree-ssa-forwprop.c) optimize this. You still have to deal with
>> non-comparison operands during expansion though, but if
>> you always forced a != 0 from the start you can then simply
>> interpret it as a proper comparison result (in which case I'd
>> modify the backends to have an alternate pattern or directly
>> expand to masking operations - using the fake comparison
>> RTX is too much of a hack).
>
> Richard, I think you didn't get the problem.
> I really need the way, to pass the information, that the expression
> that is in the first operand of vcond is an appropriate mask. I though
> for quite a while and this hack is the only answer I found, is there a
> better way to do that. I could for example introduce another
> tree-node, but it would be overkill as well.
>
> Now why do I need it so much:
> I want to implement the comparison in a way that {1, 5, 0, -1} is
> actually {-1,-1,-1,-1}. So whenever I am not sure that mask of
> VEC_COND_EXPR is a real comparison I transform it to mask != {0} (not
> always). To check the stuff, I use is_vector_comparison in
> tree-vect-generic.
>
> So I really have the difference between mask ? x : y and mask != {0} ?
> x : y, otherwise I could treat mask != {0} in the backend as just
> mask.
>
> If this link between optabs and backend breaks, then the patch falls
> apart. Because every time the comparison is taken out VEC_COND_EXPR, I
> will have to put != {0}. Keep in mind, that I cannot always put the
> comparison inside the VEC_COND_EXPR, what if it is defined in a
> function-comparison, or somehow else?
>
> So what would be an appropriate way to connect optabs and the backend?
Well, there is no problem in having the only valid mask operand for
VEC_COND_EXPRs being either a comparison or a {-1,...} / {0,....} mask.
Just the C parser has to transform mask ? vec1 : vec2 to mask != 0 ?
vec1 : vec2. This comparison can be eliminated by optimization passes
that
either replace it by the real comparison computing the mask or just
propagating the information this mask is already {-1,...} / {0,....} by simply
dropping the comparison against zero.
For the backends I'd have vcond patterns for both an embedded comparison
and for a mask. (Now we can rewind the discussion a bit and allow
arbitrary masks and define a vcond with a mask operand to do bitwise
selection - what matters is the C frontend semantics which we need to
translate to what the middle-end thinks of a VEC_COND_EXPR, they
do not have to agree).
If the mask is computed by a function you are of course out of luck,
but I don't see how you'd manage to infer knowledge from nowhere either.
Richard.
>
> Thanks,
> Artem.
>
> All the rest would be adjusted later.
>
>> tree
>> constant_boolean_node (int value, tree type)
>> {
>> - if (type == integer_type_node)
>> + if (TREE_CODE (type) == VECTOR_TYPE)
>> + {
>> + tree tval;
>> +
>> + gcc_assert (TREE_CODE (TREE_TYPE (type)) == INTEGER_TYPE);
>> + tval = build_int_cst (TREE_TYPE (type), value);
>> + return build_vector_from_val (type, tval);
>>
>> as value is either 0 or 1 that won't work. Oh, I see you pass -1
>> for true in the callers. But I think we should simply decide that true (1)
>> means -1 for a vector boolean node (and the value parameter should
>> be a bool instead). Thus,
>>
>> + if (TREE_CODE (type) == VECTOR_TYPE)
>> + {
>> + tree tval;
>> +
>> + gcc_assert (TREE_CODE (TREE_TYPE (type)) == INTEGER_TYPE);
>> + tval = build_int_cst (TREE_TYPE (type), value ? -1 : 0);
>> + return build_vector_from_val (type, tval);
>>
>> instead.
>>
>> @@ -9073,26 +9082,29 @@ fold_comparison (location_t loc, enum tr
>> floating-point, we can only do some of these simplifications.) */
>> if (operand_equal_p (arg0, arg1, 0))
>> {
>> + int true_val = TREE_CODE (type) == VECTOR_TYPE ? -1 : 0;
>> + tree arg0_type = TREE_TYPE (arg0);
>> +
>>
>> as I said this is not necessary - the FLOAT_TYPE_P and HONOR_NANS
>> macros work perfectly fine on vector types.
>>
>> Richard.
>>
>>>
>>> Thanks,
>>> Artem.
>>>
>>
>
