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?


Artem.

>> 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.
>

Reply via email to