This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Extend tree code folds to IFN_COND_*


> On Thu, May 24, 2018 at 11:36 AM Richard Sandiford
> <richard.sandiford@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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]