> On Thu, May 24, 2018 at 11:36 AM Richard Sandiford
> <richard.sandif...@linaro.org> wrote:
>>
>> This patch adds match.pd support for applying normal folds to their
>> IFN_COND_* forms. E.g. the rule:
>>
>> (plus @0 (negate @1)) -> (minus @0 @1)
>>
>> also allows the fold:
>>
>> (IFN_COND_ADD @0 @1 (negate @2) @3) -> (IFN_COND_SUB @0 @1 @2 @3)
>>
>> Actually doing this by direct matches in gimple-match.c would
>> probably lead to combinatorial explosion, so instead, the patch
>> makes gimple_match_op carry a condition under which the operation
>> happens ("cond"), and the value to use when the condition is false
>> ("else_value"). Thus in the example above we'd do the following
>>
>> (a) convert:
>>
>> cond:NULL_TREE (IFN_COND_ADD @0 @1 @4 @3) else_value:NULL_TREE
>>
>> to:
>>
>> cond:@0 (plus @1 @4) else_value:@3
>>
>> (b) apply gimple_resimplify to (plus @1 @4)
>>
>> (c) reintroduce cond and else_value when constructing the result.
>>
>> Nested operations inherit the condition of the outer operation
>> (so that we don't introduce extra faults) but have a null else_value.
>> If we try to build such an operation, the target gets to choose what
>> else_value it can handle efficiently: obvious choices include one of
>> the operands or a zero constant. (The alternative would be to have some
>> representation for an undefined value, but that seems a bit invasive,
>> and isn't likely to be useful here.)
>>
>> I've made the condition a mandatory part of the gimple_match_op
>> constructor so that it doesn't accidentally get dropped.
>>
>> Tested on aarch64-linux-gnu (with and without SVE), aarch64_be-elf
>> and x86_64-linux-gnu. OK to install?
>
> It looks somewhat clever but after looking for a while it doesn't handle
> simplifying
>
> (IFN_COND_ADD @0 @1 (IFN_COND_SUB @0 @2 @1 @3) @3)
>
> to
>
> (cond @0 @2 @3)
>
> right? Because while the conditional gimple_match_op is built
> by try_conditional_simplification it isn't built when doing
> SSA use->def following in the generated matching code?
Right. This would be easy to add, but there's no motivating case yet.
> So it looks like a bit much noise for this very special case?
>
> I suppose you ran into the need of these foldings from looking
> at real code - which foldings specifically were appearing here?
> Usually code is well optimized before if-conversion/vectorization
> so we shouldn't need full-blown handling?
It's needed to get the FMA, FMS, FNMA and FNMS folds for IFN_COND_* too.
I thought it'd be better to do it "automatically" rather than add specific
folds, since if we don't do it automatically now, it's going to end up
being a precedent for not doing it automatically in future either.
> That said, I'm not sure how much work it is to massage
>
> if (gimple *def_stmt = get_def (valueize, op2))
> {
> if (gassign *def = dyn_cast <gassign *> (def_stmt))
> switch (gimple_assign_rhs_code (def))
> {
> case PLUS_EXPR:
>
> to look like
>
> if (gimple *def_stmt = get_def (valueize, op2))
> {
> code = ERROR_MARK;
> if (!is_cond_ifn_with_cond (curr_gimple_match_op, &code))
> if (gassign *def dyn_cast <gassign *> (def_stmt))
> code = gimple_assign_rhs_code (def);
> switch (code)
> {
> case PLUS_EXPR:
>
> thus transparently treat the IFN_COND_* as their "code" if the condition
> matches that of the context (I'm not sure if we can do anything for
> mismatching contexts).
Yeah, this was one approach I had in mind for the subnodes, if we do
end up needing it. But at least for the top-level node, we want to try
both as a native IFN_COND_FOO and as a conditional FOO, which is why the
top-level case is handled directly in gimple-match-head.c.
Of course, trying both for subnodes would lead to exponential behaviour
in general. And like you say, in practice most double-IFN_COND cases
should have been folded before we created the IFN_CONDs, so it's hard
to tell which recursive behaviour would be best.
Thanks,
Richard
