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