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