On Tue, Aug 23, 2011 at 11:56 AM, Richard Guenther
<richard.guent...@gmail.com> wrote:
> 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).
Ok. So for the time being save_expr is the only way that we know to
avoid C_MAYBE_CONST_EXPR nodes.
>> 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,}>.
Richard, I think you almost get it, but there is a tiny thing you have missed.
Look, let's assume, that by some reason when we gimplified a > b, the
comparison was lifted out. So we have the following situation:
D.1 = a > b;
comp = vcond<D.1, v0, v1>
...
Ok?
Now, I fully agree that we want to treat lifted a > b as VCOND. Now,
what I am doing in the veclower is when I meet vector comparison a >
b, I wrap it in the VCOND, otherwise it would not be recognized by
optabs. literally I am doing:
rhs = gimplify_build3 (gsi, VEC_COND_EXPR, a, b, {-1}, {0}>
And here is a devil hidden. By some reason, when this expression is
gimplified, a > b is lifted again and is left outside the
VEC_COND_EXPR, and that is the problem I am trying to fight with. Have
any ideas what could be done here?
Artem.
> 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.
>>
>
