On Fri, Jul 31, 2020 at 1:47 PM Richard Biener wrote: > > On Fri, Jul 31, 2020 at 1:39 PM Richard Biener > wrote: > > > > On Fri, Jul 31, 2020 at 1:35 PM Richard Biener > > wrote: > > > > > > On Thu, Jul 30, 2020 at 9:49 AM Marc Glisse 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) > > > > " 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 > > > > > > (with > > > { > > > tree rhs1, rhs2; > > > gimple_match_op op (gimple_match_cond::UNCOND, op, > > > type, @1, @3); > > > if (op.resimplify (NULL, valueize) > > > && 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. The attached prototype works for (simplify (plus (vec_cond:s @0 @1 @2) @3) (vec_cond @0 (plus:l @1 @3) (plus:l @2 @3))) but ':...' is already taken for an explicitly specified type so I have to think about sth better. As you see I've also moved it to the actual ops that should simplify. It doesn't work on the outermost expression but I guess it doesn't make sense there (adding support would be possible). Now I need some non-ambiguous syntax... it currently is id[:type][@cid] so maybe id[!][:type][@cid]. I guess non-ambiguous is good enough? Richard. > Richard. > > > > > Richard. > > > > > The other "simple" patterns look good, you can commit > > > them separately if you like. > > > > > > Richard. > > > > > > > 2020-07-30 Marc Glisse > > > > > > > > PR tree-optimization/95906 > > > > PR target/70314 > > > > * match.pd ((c ? a : b) op d, (c ? a : b) op (c ? d : e), > > > > (v ? w : 0) ? a : b, c1 ? c2 ? a : b : b): New transformations. > > > > > > > > * gcc.dg/tree-ssa/andnot-2.c: New file. > > > > * gcc.dg/tree-ssa/pr95906.c: Likewise. > > > > * gcc.target/i386/pr70314.c: Likewise. > > > > > > > > -- > > > > Marc Glisse