| Message ID | 20240913230150.4071148-1-quic_apinski@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | phi-opt: Improve heuristics for factoring out with constant (again) [PR116699] | expand |
On Sat, Sep 14, 2024 at 1:02 AM Andrew Pinski <quic_apinski@quicinc.com> wrote: > > The heuristics for factoring out with a constant checks that the assignment statement > is the last statement of the basic block but sometimes there is a predicate or a nop statement > after the assignment. Rejecting this case does not make sense since both predicates and nop > statements are removed and don't contribute any instructions. So we should skip over them > when checking if the assignment statement was the last statement in the basic block. > > phi-opt-factor-1.c's f0 is such an example where it should catch it at phiopt1 (before predicates are removed) > and should happen in a similar way as f1 (which uses a temporary variable rather than return). OK > Bootstrapped and tested on x86_64-linux-gnu. > > PR tree-optimization/116699 > > gcc/ChangeLog: > > * tree-ssa-phiopt.cc (factor_out_conditional_operation): Skip over nop/predicates > for seeing the assignment is the last statement. > > gcc/testsuite/ChangeLog: > > * gcc.dg/tree-ssa/phi-opt-factor-1.c: New test. > > Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> > --- > .../gcc.dg/tree-ssa/phi-opt-factor-1.c | 26 +++++++++++++++++++ > gcc/tree-ssa-phiopt.cc | 6 +++++ > 2 files changed, 32 insertions(+) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-factor-1.c > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-factor-1.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-factor-1.c > new file mode 100644 > index 00000000000..12b846b9337 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-factor-1.c > @@ -0,0 +1,26 @@ > +/* { dg-options "-O2 -fdump-tree-phiopt" } */ > + > +/* PR tree-optimization/116699 > + Make sure the return PREDICT has no factor in deciding > + if we factor out the conversion. */ > + > +short f0(int a, int b, int c) > +{ > + int t1 = 4; > + if (c < t1) return (c > -1 ? c : -1); > + return t1; > +} > + > + > +short f1(int a, int b, int c) > +{ > + int t1 = 4; > + short t = t1; > + if (c < t1) t = (c > -1 ? c : -1); > + return t; > +} > + > +/* Both f1 and f0 should be optimized at phiopt1 to the same thing. */ > +/* { dg-final { scan-tree-dump-times "MAX_EXPR " 2 "phiopt1" } } */ > +/* { dg-final { scan-tree-dump-times "MIN_EXPR " 2 "phiopt1" } } */ > +/* { dg-final { scan-tree-dump-not "if " "phiopt1" } } */ > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc > index e5413e40572..7b12692237e 100644 > --- a/gcc/tree-ssa-phiopt.cc > +++ b/gcc/tree-ssa-phiopt.cc > @@ -360,6 +360,12 @@ factor_out_conditional_operation (edge e0, edge e1, gphi *phi, > } > gsi = gsi_for_stmt (arg0_def_stmt); > gsi_next_nondebug (&gsi); > + /* Skip past nops and predicates. */ > + while (!gsi_end_p (gsi) > + && (gimple_code (gsi_stmt (gsi)) == GIMPLE_NOP > + || gimple_code (gsi_stmt (gsi)) == GIMPLE_PREDICT)) > + gsi_next_nondebug (&gsi); > + /* Reject if the statement was not at the end of the block. */ > if (!gsi_end_p (gsi)) > return NULL; > } > -- > 2.43.0 >
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-factor-1.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-factor-1.c new file mode 100644 index 00000000000..12b846b9337 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-factor-1.c @@ -0,0 +1,26 @@ +/* { dg-options "-O2 -fdump-tree-phiopt" } */ + +/* PR tree-optimization/116699 + Make sure the return PREDICT has no factor in deciding + if we factor out the conversion. */ + +short f0(int a, int b, int c) +{ + int t1 = 4; + if (c < t1) return (c > -1 ? c : -1); + return t1; +} + + +short f1(int a, int b, int c) +{ + int t1 = 4; + short t = t1; + if (c < t1) t = (c > -1 ? c : -1); + return t; +} + +/* Both f1 and f0 should be optimized at phiopt1 to the same thing. */ +/* { dg-final { scan-tree-dump-times "MAX_EXPR " 2 "phiopt1" } } */ +/* { dg-final { scan-tree-dump-times "MIN_EXPR " 2 "phiopt1" } } */ +/* { dg-final { scan-tree-dump-not "if " "phiopt1" } } */ diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc index e5413e40572..7b12692237e 100644 --- a/gcc/tree-ssa-phiopt.cc +++ b/gcc/tree-ssa-phiopt.cc @@ -360,6 +360,12 @@ factor_out_conditional_operation (edge e0, edge e1, gphi *phi, } gsi = gsi_for_stmt (arg0_def_stmt); gsi_next_nondebug (&gsi); + /* Skip past nops and predicates. */ + while (!gsi_end_p (gsi) + && (gimple_code (gsi_stmt (gsi)) == GIMPLE_NOP + || gimple_code (gsi_stmt (gsi)) == GIMPLE_PREDICT)) + gsi_next_nondebug (&gsi); + /* Reject if the statement was not at the end of the block. */ if (!gsi_end_p (gsi)) return NULL; }
The heuristics for factoring out with a constant checks that the assignment statement is the last statement of the basic block but sometimes there is a predicate or a nop statement after the assignment. Rejecting this case does not make sense since both predicates and nop statements are removed and don't contribute any instructions. So we should skip over them when checking if the assignment statement was the last statement in the basic block. phi-opt-factor-1.c's f0 is such an example where it should catch it at phiopt1 (before predicates are removed) and should happen in a similar way as f1 (which uses a temporary variable rather than return). Bootstrapped and tested on x86_64-linux-gnu. PR tree-optimization/116699 gcc/ChangeLog: * tree-ssa-phiopt.cc (factor_out_conditional_operation): Skip over nop/predicates for seeing the assignment is the last statement. gcc/testsuite/ChangeLog: * gcc.dg/tree-ssa/phi-opt-factor-1.c: New test. Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> --- .../gcc.dg/tree-ssa/phi-opt-factor-1.c | 26 +++++++++++++++++++ gcc/tree-ssa-phiopt.cc | 6 +++++ 2 files changed, 32 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-factor-1.c