On Tue, Aug 23, 2011 at 12:45 PM, Artem Shinkarov
<artyom.shinkar...@gmail.com> wrote:
> On Tue, Aug 23, 2011 at 11:33 AM, Richard Guenther
> <richard.guent...@gmail.com> wrote:
>> On Tue, Aug 23, 2011 at 12:24 PM, Artem Shinkarov
>> <artyom.shinkar...@gmail.com> wrote:
>>> On Tue, Aug 23, 2011 at 11:08 AM, Richard Guenther
>>> <richard.guent...@gmail.com> wrote:
>>>> On Tue, Aug 23, 2011 at 11:44 AM, Artem Shinkarov
>>>> <artyom.shinkar...@gmail.com> wrote:
>>>>> On Tue, Aug 23, 2011 at 9:17 AM, Richard Guenther
>>>>> <richard.guent...@gmail.com> wrote:
>>>>>> On Mon, Aug 22, 2011 at 11:11 PM, Artem Shinkarov
>>>>>> <artyom.shinkar...@gmail.com> wrote:
>>>>>>> I'll just send you my current version. I'll be a little bit more
>>>>>>> specific.
>>>>>>>
>>>>>>> The problem starts when you try to lower the following expression:
>>>>>>>
>>>>>>> x = a > b;
>>>>>>> x1 = vcond <x != 0, -1, 0>
>>>>>>> vcond <x1, c, d>
>>>>>>>
>>>>>>> Now, you go from the beginning to the end of the block, and you cannot
>>>>>>> leave a > b, because only vconds are valid expressions to expand.
>>>>>>>
>>>>>>> Now, you meet a > b first. You try to transform it into vcond <a > b,
>>>>>>> -1, 0>, you build this expression, then you try to gimplify it, and
>>>>>>> you see that you have something like:
>>>>>>>
>>>>>>> x' = a >b;
>>>>>>> x = vcond <x', -1, 0>
>>>>>>> x1 = vcond <x != 0, -1, 0>
>>>>>>> vcond <x1, c, d>
>>>>>>>
>>>>>>> and your gsi stands at the x1 now, so the gimplification created a
>>>>>>> comparison that optab would not understand. And I am not really sure
>>>>>>> that you would be able to solve this problem easily.
>>>>>>>
>>>>>>> It would helpr, if you could create vcond<x, op, y, op0, op1>, but you
>>>>>>> cant and x op y is a single tree that must be gimplified, and I am not
>>>>>>> sure that you can persuade gimplifier to leave this expression
>>>>>>> untouched.
>>>>>>>
>>>>>>> In the attachment the current version of the patch.
>>>>>>
>>>>>> I can't reproduce it with your patch. For
>>>>>>
>>>>>> #define vector(elcount, type) \
>>>>>> __attribute__((vector_size((elcount)*sizeof(type)))) type
>>>>>>
>>>>>> vector (4, float) x, y;
>>>>>> vector (4, int) a,b;
>>>>>> int
>>>>>> main (int argc, char *argv[])
>>>>>> {
>>>>>> vector (4, int) i0 = x < y;
>>>>>> vector (4, int) i1 = i0 ? a : b;
>>>>>> return 0;
>>>>>> }
>>>>>>
>>>>>> I get from the C frontend:
>>>>>>
>>>>>> vector(4) int i0 = VEC_COND_EXPR < SAVE_EXPR <x < y> , { -1, -1,
>>>>>> -1, -1 } , { 0, 0, 0, 0 } > ;
>>>>>> vector(4) int i1 = VEC_COND_EXPR < SAVE_EXPR <i0> , SAVE_EXPR <a> ,
>>>>>> SAVE_EXPR <b> > ;
>>>>>>
>>>>>> but I have expected i0 != 0 in the second VEC_COND_EXPR.
>>>>>
>>>>> I don't put it there. This patch adds != 0, rather removing. But this
>>>>> could be changed.
>>>>
>>>> ?
>>>>
>>>>>> I do see that the gimplifier pulls away the condition for the first
>>>>>> VEC_COND_EXPR though:
>>>>>>
>>>>>> x.0 = x;
>>>>>> y.1 = y;
>>>>>> D.2735 = x.0 < y.1;
>>>>>> D.2734 = D.2735;
>>>>>> D.2736 = D.2734;
>>>>>> i0 = [vec_cond_expr] VEC_COND_EXPR < D.2736 , { -1, -1, -1, -1 } ,
>>>>>> { 0, 0, 0, 0 } > ;
>>>>>>
>>>>>> which is, I believe because of the SAVE_EXPR wrapped around the
>>>>>> comparison. Why do you bother wrapping all operands in save-exprs?
>>>>>
>>>>> I bother because they could be MAYBE_CONST which breaks the
>>>>> gimplifier. But I don't really know if you can do it better. I can
>>>>> always do this checking on operands of constructed vcond...
>>>>
>>>> Err, the patch does
>>>>
>>>> + /* Avoid C_MAYBE_CONST in VEC_COND_EXPR. */
>>>> + tmp = c_fully_fold (ifexp, false, &maybe_const);
>>>> + ifexp = save_expr (tmp);
>>>> + wrap &= maybe_const;
>>>>
>>>> why is
>>>>
>>>> ifexp = save_expr (tmp);
>>>>
>>>> necessary here? SAVE_EXPR is if you need to protect side-effects
>>>> from being evaluated twice if you use an operand twice. But all
>>>> operands are just used a single time.
>>>
>>> Again, the only reason why save_expr is there is to avoid MAYBE_CONST
>>> nodes to break the gimplification. But may be it is a wrong way of
>>> doing it, but it does the job.
>>>
>>>> And I expected, instead of
>>>>
>>>> + if ((COMPARISON_CLASS_P (ifexp)
>>>> + && TREE_TYPE (TREE_OPERAND (ifexp, 0)) != TREE_TYPE (op1))
>>>> + || TREE_TYPE (ifexp) != TREE_TYPE (op1))
>>>> + {
>>>> + tree comp_type = COMPARISON_CLASS_P (ifexp)
>>>> + ? TREE_TYPE (TREE_OPERAND (ifexp, 0))
>>>> + : TREE_TYPE (ifexp);
>>>> +
>>>> + op1 = convert (comp_type, op1);
>>>> + op2 = convert (comp_type, op2);
>>>> + vcond = build3 (VEC_COND_EXPR, comp_type, ifexp, op1, op2);
>>>> + vcond = convert (TREE_TYPE (op1), vcond);
>>>> + }
>>>> + else
>>>> + vcond = build3 (VEC_COND_EXPR, TREE_TYPE (op1), ifexp, op1, op2);
>>>>
>>>> if (!COMPARISON_CLASS_P (ifexp))
>>>> ifexp = build2 (NE_EXPR, TREE_TYPE (ifexp), ifexp,
>>>> build_vector_from_val (TREE_TYPE (ifexp), 0));
>>>>
>>>> if (TREE_TYPE (ifexp) != TREE_TYPE (op1))
>>>> {
>>>> ...
>>>>
>>> Why?
>>> This is a function to constuct any vcond. The result of ifexp is
>>> always signed integer vector if it is a comparison, but we need to
>>> make sure that all the elements of vcond have the same type.
>>>
>>> And I didn't really understand if we can guarantee that vector
>>> comparison would not be lifted out by the gimplifier. It happens in
>>> case I put this save_expr, it could possibly happen in some other
>>> cases. How can we prevent that?
>>
>> We don't need to prevent it. If the C frontend makes sure that the
>> mask of a VEC_COND_EXPR is always {-1,...} or {0,....} by expanding
>> mask ? v1 : v2 to VEC_COND_EXPR <mask != 0, v1, v2> then
>> the expansion can do the obvious thing with a non-comparison mask
>> (have another md pattern for this case to handle AMD XOP vcond
>> or simply emit bitwise mask operations).
>>
>> The gimplifier shouldn't unnecessarily pull out the comparison, but
>> you instructed it to - by means of wrapping it inside a SAVE_EXPR.
>>
>> Richard.
>>
>
> I'm confused.
> There is a set of problems which are tightly connected and you address
> only one one of them.
>
> I need to do something with C_MAYBE_CONST_EXPR node to allow the
> gimplification of the expression. In order to achieve that I am
> wrapping expression which can contain C_MAYBE_EXPR_NODE into
> SAVE_EXPR. This works fine, but, the vector condition is lifted out.
> So the question is how to get rid of C_MAYBE_CONST_EXPR nodes, making
> sure that the expression is still inside VEC_COND_EXPR?
I can't answer this, but no C_MAYBE_CONST_EXPR nodes may survive
until gimplification. I thought c_fully_fold is exactly used (instead
of c_save_expr) because it _doesn't_ wrap things in C_MAYBE_CONST_EXPR
nodes. Instead you delay that (well, commented out in your patch).
> All the rest is fine -- a > b is transformed to VEC_COND_EXPR of the
> integer type, and when we are using it we can add != 0 to the mask, no
> problem. The problem is to make sure that the vector expression is not
> lifted out from the VEC_COND_EXPR and that C_MAYBE_CONST_EXPRs are
> also no there at the same time.
Well, for example for floating-point comparisons and -fnon-call-exceptions
you _will_ get comparisons lifted out of the VEC_COND_EXPR. But
that shouldn't be an issue because C semantics are ensured for
the mask ? v0 : v1 source form by changing it to mask != 0 ? v0 : v1 and
the VEC_COND_EXPR semantic for a non-comparison mask operand
is (v0 & mask) | (v1 & ~mask). Which means that we have to be able to
expand mask = v0 < v1 anyway, but we'll simply expand it if it were
VEC_COND_EXPR <v0<v1, {-1,}, {0,}>.
So, I don't really see any problems for the C frontend or gimplification side.
We do have to make expansion handle more cases, but they can be all
dispatched to make use of the vcond named expander and handling
the mask ? v1 : v2 case with bitwise operations (to be optimized later
by introducing another named expander to match XOP vcond).
Richard.
> Artem.
>
