>> I don't know much about valueisation either :) But it does feel >> like we're working around the lack of a LEN form of COND_EXPR. >> In other words, it seems odd that we can do: >> >> IFN_COND_LEN_ADD (mask, a, 0, b, len, bias) >> >> but we can't do: >> >> IFN_COND_LEN (mask, a, b, len, bias) >> >> There seems to be no way of applying a length without also finding an >> operation to perform. > > Indeed .. maybe - _maybe_ we want to scrap VEC_COND_EXPR for > IFN_COND{,_LEN} to be more consistent here?
So, yes we could define IFN_COND_LEN (or VCOND_MASK_LEN) but I'd assume that there would be a whole lot of follow-up things to consider. I'm wondering if we really gain something from the the round-trip via VEC_COND_EXPR when we eventually create a COND_(LEN_)_OP anyway? Sure, if the target doesn't have the particular operation we would want a VEC_COND_EXPR. Same if SEQ is somehow more complicated. So the IFN_COND(_LEN) =? VCOND_MASK(_LEN) discussion notwithstanding, couldn't what I naively proposed be helpful as well? Or do we potentially lose optimizations during the time where e.g. a _foo = a BINOP b VEC_COND_EXPR (cond, foo, else) has not yet been converted into a COND_OP? We already create COND_OPs for the other paths (via convert_conditional_op) so why not for this one? Or am I missing some interdependence with SEQ? FWIW I did a full bootstrap and testsuite run on the usual architectures showing no changes with the attached patch. Regards Robin Subject: [PATCH] gimple-match: Create COND_OP directly if possible. This patch converts simplified sequences into conditional operations instead of VEC_COND_EXPRs if the target supports them. This helps for len-masked targets which cannot directly use a VEC_COND_EXPR in the presence of length masking. gcc/ChangeLog: * gimple-match-exports.cc (directly_supported_p): Define. (maybe_resimplify_conditional_op): Create COND_OP directly. * gimple-match.h (gimple_match_cond::gimple_match_cond): Initialize length and bias. --- gcc/gimple-match-exports.cc | 40 ++++++++++++++++++++++++++++--------- gcc/gimple-match.h | 7 +++++-- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/gcc/gimple-match-exports.cc b/gcc/gimple-match-exports.cc index b36027b0bad..ba3bd1450db 100644 --- a/gcc/gimple-match-exports.cc +++ b/gcc/gimple-match-exports.cc @@ -98,6 +98,8 @@ static bool gimple_resimplify5 (gimple_seq *, gimple_match_op *, tree (*)(tree)) static bool gimple_resimplify6 (gimple_seq *, gimple_match_op *, tree (*)(tree)); static bool gimple_resimplify7 (gimple_seq *, gimple_match_op *, tree (*)(tree)); +bool directly_supported_p (code_helper, tree, optab_subtype); + /* Match and simplify the toplevel valueized operation THIS. Replaces THIS with a simplified and/or canonicalized result and returns whether any change was made. */ @@ -299,22 +301,42 @@ maybe_resimplify_conditional_op (gimple_seq *seq, gimple_match_op *res_op, } } - /* If the "then" value is a gimple value and the "else" value matters, - create a VEC_COND_EXPR between them, then see if it can be further - simplified. */ + /* If the condition represents MASK ? THEN : ELSE, where THEN is a gimple + value and ELSE matters, create a VEC_COND_EXPR between them, then see + if it can be further simplified. + For COND_LEN masking, try to create a COND_LEN_OP directly in case + SEQ contains a supportable operation. */ gimple_match_op new_op; if (res_op->cond.else_value && VECTOR_TYPE_P (res_op->type) && gimple_simplified_result_is_gimple_val (res_op)) { - new_op.set_op (VEC_COND_EXPR, res_op->type, - res_op->cond.cond, res_op->ops[0], - res_op->cond.else_value); - *res_op = new_op; - return gimple_resimplify3 (seq, res_op, valueize); + /* If a previous simplification was pushed to SEQ + and we can convert it to a COND_OP directly, do so + in order to save a round-trip via VEC_COND_EXPR -> COND_OP. */ + if (seq && *seq && is_gimple_assign (*seq) + && directly_supported_p (gimple_assign_rhs_code (*seq), res_op->type, + optab_scalar)) + { + res_op->code = gimple_assign_rhs_code (*seq); + res_op->num_ops = gimple_num_ops (*seq) - 1; + res_op->ops[0] = gimple_assign_rhs1 (*seq); + if (res_op->num_ops > 1) + res_op->ops[1] = gimple_assign_rhs2 (*seq); + if (res_op->num_ops > 2) + res_op->ops[2] = gimple_assign_rhs2 (*seq); + } + else if (!res_op->cond.len) + { + new_op.set_op (VEC_COND_EXPR, res_op->type, + res_op->cond.cond, res_op->ops[0], + res_op->cond.else_value); + *res_op = new_op; + return gimple_resimplify3 (seq, res_op, valueize); + } } - /* Otherwise try rewriting the operation as an IFN_COND_* call. + /* Otherwise try rewriting the operation as an IFN_COND_(LEN_)* call. Again, this isn't a simplification in itself, since it's what RES_OP already described. */ if (convert_conditional_op (res_op, &new_op)) diff --git a/gcc/gimple-match.h b/gcc/gimple-match.h index bec3ff42e3e..55c771d560f 100644 --- a/gcc/gimple-match.h +++ b/gcc/gimple-match.h @@ -32,7 +32,9 @@ public: enum uncond { UNCOND }; /* Build an unconditional op. */ - gimple_match_cond (uncond) : cond (NULL_TREE), else_value (NULL_TREE) {} + gimple_match_cond (uncond) + : cond (NULL_TREE), else_value (NULL_TREE), len (NULL_TREE), + bias (NULL_TREE) {} gimple_match_cond (tree, tree); gimple_match_cond (tree, tree, tree, tree); @@ -56,7 +58,8 @@ public: inline gimple_match_cond::gimple_match_cond (tree cond_in, tree else_value_in) - : cond (cond_in), else_value (else_value_in) + : cond (cond_in), else_value (else_value_in), len (NULL_TREE), + bias (NULL_TREE) { } -- 2.41.0