| Message ID | 20240501032114.1112795-1-quic_apinski@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | MATCH: Add some more value_replacement simplifications (a != 0 ? expr : 0) to match | expand |
On 4/30/24 9:21 PM, Andrew Pinski wrote: > This adds a few more of what is currently done in phiopt's value_replacement > to match. I noticed this when I was hooking up phiopt's value_replacement > code to use match and disabling the old code. But this can be done > independently from the hooking up phiopt's value_replacement as phiopt > is already hooked up for simplified versions already. > > /* a != 0 ? a / b : 0 -> a / b iff b is nonzero. */ > /* a != 0 ? a * b : 0 -> a * b */ > /* a != 0 ? a & b : 0 -> a & b */ > > We prefer the `cond ? a : 0` forms to allow optimization of `a * cond` which > uses that form. > > Bootstrapped and tested on x86_64-linux-gnu with no regressions. > > PR treee-optimization/114894 > > gcc/ChangeLog: > > * match.pd (`a != 0 ? a / b : 0`): New pattern. > (`a != 0 ? a * b : 0`): New pattern. > (`a != 0 ? a & b : 0`): New pattern. > > gcc/testsuite/ChangeLog: > > * gcc.dg/tree-ssa/phi-opt-value-5.c: New test. Is there any need to also handle the reversed conditional with the arms swapped? If not, this is fine as-is. If yes, then fine with the obvious generalization. jeff
On Tue, May 7, 2024 at 1:45 PM Jeff Law <jeffreyalaw@gmail.com> wrote: > > > > On 4/30/24 9:21 PM, Andrew Pinski wrote: > > This adds a few more of what is currently done in phiopt's value_replacement > > to match. I noticed this when I was hooking up phiopt's value_replacement > > code to use match and disabling the old code. But this can be done > > independently from the hooking up phiopt's value_replacement as phiopt > > is already hooked up for simplified versions already. > > > > /* a != 0 ? a / b : 0 -> a / b iff b is nonzero. */ > > /* a != 0 ? a * b : 0 -> a * b */ > > /* a != 0 ? a & b : 0 -> a & b */ > > > > We prefer the `cond ? a : 0` forms to allow optimization of `a * cond` which > > uses that form. > > > > Bootstrapped and tested on x86_64-linux-gnu with no regressions. > > > > PR treee-optimization/114894 > > > > gcc/ChangeLog: > > > > * match.pd (`a != 0 ? a / b : 0`): New pattern. > > (`a != 0 ? a * b : 0`): New pattern. > > (`a != 0 ? a & b : 0`): New pattern. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.dg/tree-ssa/phi-opt-value-5.c: New test. > Is there any need to also handle the reversed conditional with the arms > swapped? If not, this is fine as-is. If yes, then fine with the > obvious generalization. The answer is yes and no. While the PHI-OPT pass will try both cases but the other (all?) passes does not. This is something I have been thinking about trying to solve in a generic way instead of adding many more patterns here. I will start working on that in the middle of June. Most of the time cond patterns in match are used is inside phiopt so having the revered conditional has not been on high on my priority but with VRP and scev and match (itself) producing more cond_expr, we should fix this once and for all for GCC 15. Thanks, Andrew Pinski > > jeff >
On Tue, May 7, 2024 at 10:56 PM Andrew Pinski <pinskia@gmail.com> wrote: > > On Tue, May 7, 2024 at 1:45 PM Jeff Law <jeffreyalaw@gmail.com> wrote: > > > > > > > > On 4/30/24 9:21 PM, Andrew Pinski wrote: > > > This adds a few more of what is currently done in phiopt's value_replacement > > > to match. I noticed this when I was hooking up phiopt's value_replacement > > > code to use match and disabling the old code. But this can be done > > > independently from the hooking up phiopt's value_replacement as phiopt > > > is already hooked up for simplified versions already. > > > > > > /* a != 0 ? a / b : 0 -> a / b iff b is nonzero. */ > > > /* a != 0 ? a * b : 0 -> a * b */ > > > /* a != 0 ? a & b : 0 -> a & b */ > > > > > > We prefer the `cond ? a : 0` forms to allow optimization of `a * cond` which > > > uses that form. > > > > > > Bootstrapped and tested on x86_64-linux-gnu with no regressions. > > > > > > PR treee-optimization/114894 > > > > > > gcc/ChangeLog: > > > > > > * match.pd (`a != 0 ? a / b : 0`): New pattern. > > > (`a != 0 ? a * b : 0`): New pattern. > > > (`a != 0 ? a & b : 0`): New pattern. > > > > > > gcc/testsuite/ChangeLog: > > > > > > * gcc.dg/tree-ssa/phi-opt-value-5.c: New test. > > Is there any need to also handle the reversed conditional with the arms > > swapped? If not, this is fine as-is. If yes, then fine with the > > obvious generalization. > > The answer is yes and no. While the PHI-OPT pass will try both cases > but the other (all?) passes does not. This is something I have been > thinking about trying to solve in a generic way instead of adding many > more patterns here. I will start working on that in the middle of > June. > Most of the time cond patterns in match are used is inside phiopt so > having the revered conditional has not been on high on my priority but > with VRP and scev and match (itself) producing more cond_expr, we > should fix this once and for all for GCC 15. IMO this is a classical case for canonicalization. IIRC in fold we rely on tree_swap_operands_p for the COND_EXPR arms and if we can invert the condition we do so. So there's a conflict of interest with respect to condition canonicalization and true/false canonicalization. We do not canonicalize COND_EXPRs in gimple_resimplify3, but the only natural thing there would be to do it based on the op2/op3 operands, looking at the conditional would dive down one level too deep. Richard. > Thanks, > Andrew Pinski > > > > > jeff > >
diff --git a/gcc/match.pd b/gcc/match.pd index d401e7503e6..03a03c31233 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -4290,6 +4290,24 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) (cond (eq @0 integer_all_onesp) @1 (op:c@2 @1 @0)) @2)) +/* a != 0 ? a / b : 0 -> a / b iff b is nonzero. */ +(for op (trunc_div ceil_div floor_div round_div exact_div) + (simplify + (cond (ne @0 integer_zerop) (op@2 @3 @1) integer_zerop ) + (if (bitwise_equal_p (@0, @3) + && tree_expr_nonzero_p (@1)) + @2))) + +/* Note we prefer the != case here + as (a != 0) * (a * b) will generate that version. */ +/* a != 0 ? a * b : 0 -> a * b */ +/* a != 0 ? a & b : 0 -> a & b */ +(for op (mult bit_and) + (simplify + (cond (ne @0 integer_zerop) (op:c@2 @1 @3) integer_zerop) + (if (bitwise_equal_p (@0, @3)) + @2))) + /* Simplifications of shift and rotates. */ (for rotate (lrotate rrotate) diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-value-5.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-value-5.c new file mode 100644 index 00000000000..8062eb19b11 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-value-5.c @@ -0,0 +1,39 @@ +/* { dg-do compile } */ +/* PR treee-optimization/114894 */ +/* Phi-OPT should be able to optimize these without sinking being invoked. */ +/* { dg-options "-O -fdump-tree-phiopt2 -fdump-tree-phiopt3 -fdump-tree-optimized -fno-tree-sink" } */ + +int fmul1(int a, int b) +{ + int c = a * b; + if (a != 0) + return c; + return 0; +} + + +int fand1(int a, int b) +{ + int c = a & b; + if (a != 0) + return c; + return 0; +} + + +void g(int); + +int fdiv1(int a, int b) +{ + int d = b|1; + g(d); + int c = a / d; + return a != 0 ? c : 0; +} + +/* fdiv1 requires until later than phiopt2 to be able to detect that + d is non-zero. to be able to remove the conditional. */ +/* { dg-final { scan-tree-dump-times "goto" 2 "phiopt2" } } */ +/* { dg-final { scan-tree-dump-not "goto" "phiopt3" } } */ +/* { dg-final { scan-tree-dump-not "goto" "optimized" } } */ +
This adds a few more of what is currently done in phiopt's value_replacement to match. I noticed this when I was hooking up phiopt's value_replacement code to use match and disabling the old code. But this can be done independently from the hooking up phiopt's value_replacement as phiopt is already hooked up for simplified versions already. /* a != 0 ? a / b : 0 -> a / b iff b is nonzero. */ /* a != 0 ? a * b : 0 -> a * b */ /* a != 0 ? a & b : 0 -> a & b */ We prefer the `cond ? a : 0` forms to allow optimization of `a * cond` which uses that form. Bootstrapped and tested on x86_64-linux-gnu with no regressions. PR treee-optimization/114894 gcc/ChangeLog: * match.pd (`a != 0 ? a / b : 0`): New pattern. (`a != 0 ? a * b : 0`): New pattern. (`a != 0 ? a & b : 0`): New pattern. gcc/testsuite/ChangeLog: * gcc.dg/tree-ssa/phi-opt-value-5.c: New test. Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> --- gcc/match.pd | 18 +++++++++ .../gcc.dg/tree-ssa/phi-opt-value-5.c | 39 +++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-value-5.c