Re: Vector Comparison patch

[Richard Guenther <richard dot guenther at gmail dot com>]

On Fri, Aug 19, 2011 at 5:22 PM, Artem Shinkarov
<artyom.shinkaroff@gmail.com> wrote:
> On Fri, Aug 19, 2011 at 3:54 PM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Fri, Aug 19, 2011 at 2:29 AM, Artem Shinkarov
>> <artyom.shinkaroff@gmail.com> wrote:
>>> Hi, I had the problem with passing information about single variable
>>> from expand_vec_cond_expr optab into ix86_expand_*_vcond.
>>>
>>> I looked into it this problem for quite a while and found a solution.
>>> Now the question if it could be done better.
>>>
>>> First of all the problem:
>>>
>>> If we represent any vector comparison with VEC_COND_EXPR < v0 <OP> v1
>>> ? {-1,...} : {0,...} >, then in the assembler we do not want to see
>>> this useless comparison with {-1...}.
>>>
>>> Now it is easy to fix the problem about excessive masking. The real
>>> challenge starts when the comparison inside vcond is expressed as a
>>> variable. In that case in order to construct correct vector expression
>>> we need to adjust cond in cond ? v0 : v1 to ?cond == {-1...} or as we
>>> agreed recently cond != {0,..}. But hat we need to do only to make
>>> vec_cond_expr happy. On the level of assembler we don't want this
>>> condition.
>>>
>>> Now, if I just construct the tree, then in x86, rtx_equal_p, does not
>>> know that this is a constant vector full of -1, because the comparison
>>> operands are not immediate. So I need somehow to mark the fact in
>>> optabs, and then check the information in the x86.
>>
>> Well, this is why I was suggesting the bitwise semantic for a mask
>> operand. ?What we should do on the tree level (and that should happen
>> already), is forward the comparison into the COND_EXPR. ?Thus,
>>
>> mask = v1 < v2;
>> v3 = mask ? v4 : v5;
>>
>> should get changed to
>>
>> v3 = v1 < v2 ? v4 : v5;
>>
>> by tree-ssa-forwprop.c. ?If that is not happening we have to fix that there.
>
> Yeah, that is something I am working on.
>
>> Because we _don't_ know the mask is all -1 or 0 ;) ?The user might
>> put in {3, 5 ,1 3} and expect it to be treated like {-1,...} but it isn't
>> so already.
>>
>>> At the moment I do something like this:
>>>
>>> optabs:
>>>
>>> if (!COMPARISON_CLASS_P (op0))
>>> ?ops[3] = gen_rtx_EQ (mode, NULL_RTX, NULL_RTX);
>>>
>>> This expression is preserved while checking and verifying.
>>>
>>> ix86:
>>> if (GET_CODE (comp) == EQ && XEXP (comp, 0) == NULL_RTX
>>> ? ? ?&& XEXP (comp, 1) == NULL_RTX)
>>>
>>> See the patch attached for more details. The patch is just to give you
>>> an idea of the way I am doing it and it seems to work. Please don't
>>> criticise the patch itself, better help me to understand if there is a
>>> better way to pass the information from optabs to ix86.
>>
>> Hm, I'm not sure the expand_vec_cond_expr will work that way,
>> I'd have to play with it myself (but will now be running for weekend).
>>
>> Is the special-casing of a < b ? {-1,-1,-1} : {0,0,0,0} in the backend
>> working for you? ?I think there are probably some rtl all-ones and all-zeros
>> predicates you can re-use.
>>
>> Richard.
>
> It works fine. Masks all ones and all zeroes are predefined, all -1
> are not, but I am switching to all zeroes. The real question is that

All -1 is the same as all ones.

> this special case of comparison with two empty operands is a little
> bit hackish. On the other hand there should be no problem with that,

I didn't mean this special case which I believe is incorrect anyways
due to the above comment, but the special case resulting from
expanding v1 < v2 as v1 < v2 ? {-1,-1...} : {0,0,...}.

> because operand 3 is used only to get the code of comparison, noone is
> looking inside the arguments, so we could use this fact. The question
> is whether there is a better way.

As I said above, we can't rely on the mask being either {-1,...} or {0,...}.
If we can, then we should have propagated a comparison, otherwise
we need a real != compare with { 0,....}.

> Thanks,
> Artem.
>