On Thu, Apr 17, 2014 at 3:09 PM, Yuri Rumyantsev <ysrum...@gmail.com> wrote: > Hi All, > > We implemented enhancement for if-convert phase to recognize the > simplest conditional reduction and to transform it vectorizable form, > e.g. statement > if (A[i] != 0) num+= 1; will be recognized. > A new test-case is also provided. > > Bootstrapping and regression testing did not show any new failures.
Clever. Can you add a testcase with a non-constant but invariant reduction value and one with a variable reduction value as well? + if (!(is_cond_scalar_reduction (arg_0, &reduc, &op0, &op1) + || is_cond_scalar_reduction (arg_1, &reduc, &op0, &op1))) Actually one of the args should be defined by a PHI node in the loop header and the PHI result should be the PHI arg on the latch edge, so I'd pass both PHI args to the predicate and do the decision on what the reduction op is there (you do that anyway). The pattern matching is somewhat awkward + /* Consider only conditional reduction. */ + bb = gimple_bb (stmt); + if (!bb_has_predicate (bb)) + return false; + if (is_true_predicate (bb_predicate (bb))) + return false; should be replaced by matching the PHI structure loop-header: reduc_1 = PHI <..., reduc_2> ... if (..) reduc_3 = ... reduc_2 = PHI <reduc_1, reduc_3> + lhs = gimple_assign_lhs (stmt); + if (TREE_CODE (lhs) != SSA_NAME) + return false; always true, in fact lhs == arg. + if (SSA_NAME_VAR (lhs) == NULL) + return false; no need to check that (or later verify SSA_NAME_VAR equivalency), not sure why you think you need that. + if (!single_imm_use (lhs, &use, &use_stmt)) + return false; + if (gimple_code (use_stmt) != GIMPLE_PHI) + return false; checking has_single_use (arg) is enough. The above is error-prone wrt debug statements. + if (reduction_op == PLUS_EXPR && + TREE_CODE (r_op2) == SSA_NAME) && goes to the next line + if (TREE_CODE (r_op2) != INTEGER_CST && TREE_CODE (r_op2) != REAL_CST) + return false; any reason for this check? The vectorizer can cope with loop invariant non-constant values as well (at least). + /* Right operand is constant, check that left operand is equal to lhs. */ + if (SSA_NAME_VAR (lhs) != SSA_NAME_VAR (r_op1)) + return false; see above - that looks weird. Note that I think you may introduce undefined overflow in unconditionally executing the increment. So you need to make sure to re-write the increment in unsigned arithmetic (for integral types, that is). Thanks, Richard. > Is it OK for trunk? > > gcc/ChangeLog: > 2014-04-17 Yuri Rumyantsev <ysrum...@gmail.com> > > * tree-if-conv.c (is_cond_scalar_reduction): New function. > (convert_scalar_cond_reduction): Likewise. > (predicate_scalar_phi): Add recognition and transformation > of simple conditioanl reduction to be vectorizable. > > gcc/testsuite/ChangeLog: > 2014-04-17 Yuri Rumyantsev <ysrum...@gmail.com> > > * gcc.dg/cond-reduc.c: New test.