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.
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))
{
...
> You are right, that if you just put a comparison of variables there
> then we are fine. My point is that whenever gimplifier is pulling out
> the comparison from the first operand, replacing it with the variable,
> then we are screwed, because there is no chance to put it back, and
> that is exactly what happens in expand_vector_comparison, if you
> uncomment the replacement -- comparison is always represented as x = a
>> b.
>
>> With that the
>>
>> /* Currently the expansion of VEC_COND_EXPR does not allow
>> expessions where the type of vectors you compare differs
>> form the type of vectors you select from. For the time
>> being we insert implicit conversions. */
>> if ((COMPARISON_CLASS_P (ifexp)
>> && TREE_TYPE (TREE_OPERAND (ifexp, 0)) != TREE_TYPE (op1))
>> || TREE_TYPE (ifexp) != TREE_TYPE (op1))
>>
>> checks will fail (because ifexp is a SAVE_EXPR).
>>
>> I'll run into errors when not adding the SAVE_EXPR around the ifexp,
>> the transform into x < y ? {-1,...} : {0,...} is not happening.
