On Fri, 31 Jul 2020, Richard Biener wrote:

On Fri, Jul 31, 2020 at 1:39 PM Richard Biener
<richard.guent...@gmail.com> wrote:

On Fri, Jul 31, 2020 at 1:35 PM Richard Biener
<richard.guent...@gmail.com> wrote:

On Thu, Jul 30, 2020 at 9:49 AM Marc Glisse <marc.gli...@inria.fr> wrote:

When vector comparisons were forced to use vec_cond_expr, we lost a number
of optimizations (my fault for not adding enough testcases to prevent
that). This patch tries to unwrap vec_cond_expr a bit so some
optimizations can still happen.

I wasn't planning to add all those transformations together, but adding
one caused a regression, whose fix introduced a second regression, etc.

Using a simple fold_binary internally looks like an ok compromise to me.
It remains cheap enough (not recursive, and vector instructions are not
that frequent), while still allowing more than const_binop (X|0 or X&X for
instance). The transformations are quite conservative with :s and folding
only if everything simplifies, we may want to relax this later. And of
course we are going to miss things like a?b:c + a?c:b -> b+c.

In terms of number of operations, some transformations turning 2
VEC_COND_EXPR into VEC_COND_EXPR + BIT_IOR_EXPR + BIT_NOT_EXPR might not
look like a gain... I expect the bit_not disappears in most cases, and
VEC_COND_EXPR looks more costly than a simpler BIT_IOR_EXPR.

I am a bit confused that with avx512 we get types like "vector(4)
<signed-boolean:2>" with :2 and not :1 (is it a hack so true is 1 and not
-1?), but that doesn't matter for this patch.

Regtest+bootstrap on x86_64-pc-linux-gnu

+  (with
+   {
+     tree rhs1, rhs2 = NULL;
+     rhs1 = fold_binary (op, type, @1, @3);
+     if (rhs1 && is_gimple_val (rhs1))
+       rhs2 = fold_binary (op, type, @2, @3);

ICK.  I guess a more match-and-simplify way would be

I was focused on avoiding recursion, but indeed that's independent, I could have set a trivial valueize function for that.

   (with
    {
      tree rhs1, rhs2;
      gimple_match_op op (gimple_match_cond::UNCOND, op,
                                      type, @1, @3);
      if (op.resimplify (NULL, valueize)

Oh, so you would be ok with something that recurses without limit? I thought we were avoiding this because it may allow some compile time explosion with pathological examples.

          && gimple_simplified_result_is_gimple_val (op))
       {
         rhs1 = op.ops[0];
         ... other operand ...
       }

now in theory we could invent some new syntax for this, like

 (simplify
  (op (vec_cond:s @0 @1 @2) @3)
  (vec_cond @0 (op:x @1 @3) (op:x @2 @3)))

and pick something better instead of :x (:s is taken,
would be 'simplified', :c is taken would be 'constexpr', ...).

_Maybe_ just

 (simplify
  (op (vec_cond:s @0 @1 @2) @3)
  (vec_cond:x @0 (op @1 @3) (op @2 @3)))

which would have the same practical meaning as passing
NULL for the seq argument to simplification - do not allow
any intermediate stmt to be generated.

Note I specifically do not like those if (it-simplifies) checks
because we already would code-generate those anyway.  For

(simplify
  (plus (vec_cond:s @0 @1 @2) @3)
  (vec_cond @0 (plus @1 @3) (plus @2 @3)))

we get

                    res_op->set_op (VEC_COND_EXPR, type, 3);
                    res_op->ops[0] = captures[1];
                    res_op->ops[0] = unshare_expr (res_op->ops[0]);
                    {
                      tree _o1[2], _r1;
                      _o1[0] = captures[2];
                      _o1[1] = captures[4];
                      gimple_match_op tem_op (res_op->cond.any_else
(), PLUS_EXPR, TREE_TYPE (_o1[0]), _o1[0], _o1[1]);
                      tem_op.resimplify (lseq, valueize);
                      _r1 = maybe_push_res_to_seq (&tem_op, lseq);  (****)
                      if (!_r1) return false;
                      res_op->ops[1] = _r1;
                    }
                    {
                      tree _o1[2], _r1;
                      _o1[0] = captures[3];
                      _o1[1] = captures[4];
                      gimple_match_op tem_op (res_op->cond.any_else
(), PLUS_EXPR, TREE_TYPE (_o1[0]), _o1[0], _o1[1]);
                      tem_op.resimplify (lseq, valueize);
                      _r1 = maybe_push_res_to_seq (&tem_op, lseq);  (***)
                      if (!_r1) return false;
                      res_op->ops[2] = _r1;
                    }
                    res_op->resimplify (lseq, valueize);
                    return true;

and the only change required would be to pass NULL to maybe_push_res_to_seq
here instead of lseq at the (***) marked points.

(simplify
 (plus (vec_cond:s @0 @1 @2) @3)
 (vec_cond:l @0 (plus @1 @3) (plus @2 @3)))

'l' for 'force leaf'.  I'll see if I can quickly cme up with a patch.

Does it have a clear meaning for GENERIC? I guess that's probably not such a big problem.

There are a number of transformations that we would like to perform "if <something> simplifies", but I don't know if it will always have exactly this form of "if this part of the output pattern doesn't simplify, give up". In some cases, we might prefer "ok if at least one of the branches simplifies" for instance. Or "ok if this generates at most one extra statement". Even for this particular transformation, I am not sure this is the right condition.

Your suggestion looks good (although I understand better the version with a mark on plus instead of vec_cond_expr), I just want to avoid wasting your time if it turns out we don't use it that much in the end (I have no idea yet).

--
Marc Glisse

Reply via email to