| Message ID | 20240930215010.768869-1-quic_apinski@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | phi-opt: Improve factor heurstic with constants and conversions from bool [PR116890] | expand |
On Mon, Sep 30, 2024 at 11:50 PM Andrew Pinski <quic_apinski@quicinc.com> wrote: > > Take: > ``` > if (t_3(D) != 0) > goto <bb 3>; > else > goto <bb 4>; > > <bb 3> > _8 = c_4(D) != 0; > _9 = (int) _8; > > <bb 4> > # e_2 = PHI <_9(3), 0(2)> > ``` > > We should factor out the conversion here as that will allow a simplfication to > `(t_3 != 0) & (c_4 != 0)`. Unlike most other types; `a ? b : CST` will simplify > for boolean result type to either `a | b` or `a & b` so allowing this conversion > for all operations will be always profitable. > > Bootstrapped and tested on x86_64-linux-gnu with no regressions. > > Note on the phi-opt-7.c testcase change, we are now able to optimize this > and remove the if due to the factoring out now so this is an improvement. OK. Thanks, Richard. > PR tree-optimization/116890 > > gcc/ChangeLog: > > * tree-ssa-phiopt.cc (factor_out_conditional_operation): Conversions > from bool is also should be considered as wanting to happen. > > gcc/testsuite/ChangeLog: > > * gcc.dg/tree-ssa/phi-opt-7.c: Update testcase for no ifs left. > * gcc.dg/tree-ssa/phi-opt-42.c: New test. > * gcc.dg/tree-ssa/phi-opt-43.c: New test. > > Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> > --- > gcc/testsuite/gcc.dg/tree-ssa/phi-opt-42.c | 19 +++++++++++++++++++ > gcc/testsuite/gcc.dg/tree-ssa/phi-opt-43.c | 19 +++++++++++++++++++ > gcc/testsuite/gcc.dg/tree-ssa/phi-opt-7.c | 6 +++--- > gcc/tree-ssa-phiopt.cc | 10 +++++++++- > 4 files changed, 50 insertions(+), 4 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-42.c > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-43.c > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-42.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-42.c > new file mode 100644 > index 00000000000..62556945159 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-42.c > @@ -0,0 +1,19 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O1 -fdump-tree-optimized" } */ > + > +/* PR tree-optimization/116890 */ > + > +int f(int a, int b, int c) > +{ > + int x; > + if (c) x = a == 0; > + else x = 0; > + return x; > +} > + > + > +/* The if should have been removed as the the conversion from bool to int should have been factored out. */ > +/* { dg-final { scan-tree-dump-not "if" "optimized" } }*/ > +/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = a_\[0-9\]*.D. == 0" 1 "optimized" } } */ > +/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = c_\[0-9\]*.D. != 0" 1 "optimized" } } */ > + > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-43.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-43.c > new file mode 100644 > index 00000000000..1d16f283f27 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-43.c > @@ -0,0 +1,19 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O1 -fdump-tree-optimized" } */ > + > +/* PR tree-optimization/116890 */ > + > +int f(_Bool a, _Bool b, int c) > +{ > + int x; > + if (c) x = a & b; > + else x = 0; > + return x; > +} > + > + > +/* The if should have been removed as the the conversion from bool to int should have been factored out. */ > +/* { dg-final { scan-tree-dump-not "if" "optimized" } }*/ > +/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = a_\[0-9\]*.D. & b_\[0-9\]*.D." 1 "optimized" } } */ > +/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = c_\[0-9\]*.D. != 0" 1 "optimized" } } */ > + > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-7.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-7.c > index 51e1f6dfa75..3ee43e55692 100644 > --- a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-7.c > +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-7.c > @@ -15,8 +15,8 @@ int f(int t, int c) > return g(d,e); > } > > -/* There should be one ifs as one of them should be changed into > - a conditional and the other should be there still. */ > -/* { dg-final { scan-tree-dump-times "if" 1 "optimized" } }*/ > +/* There should be no ifs as this is converted into `(t != 0) & (c != 0)`. > +/* { dg-final { scan-tree-dump-not "if" "optimized" } }*/ > /* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = c_\[0-9\]*.D. != 0" 1 "optimized" } } */ > +/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = t_\[0-9\]*.D. != 0" 1 "optimized" } } */ > > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc > index d43832b390b..bd7f9607eb9 100644 > --- a/gcc/tree-ssa-phiopt.cc > +++ b/gcc/tree-ssa-phiopt.cc > @@ -345,9 +345,17 @@ factor_out_conditional_operation (edge e0, edge e1, gphi *phi, > if (gassign *assign = dyn_cast <gassign *> (stmt)) > { > tree lhs = gimple_assign_lhs (assign); > + tree lhst = TREE_TYPE (lhs); > enum tree_code ass_code > = gimple_assign_rhs_code (assign); > - if (ass_code != MAX_EXPR && ass_code != MIN_EXPR) > + if (ass_code != MAX_EXPR && ass_code != MIN_EXPR > + /* Conversions from boolean like types is ok > + as `a?1:b` and `a?0:b` will always simplify > + to `a & b` or `a | b`. > + See PR 116890. */ > + && !(INTEGRAL_TYPE_P (lhst) > + && TYPE_UNSIGNED (lhst) > + && TYPE_PRECISION (lhst) == 1)) > return NULL; > if (lhs != gimple_assign_rhs1 (arg0_def_stmt)) > return NULL; > -- > 2.34.1 >
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-42.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-42.c new file mode 100644 index 00000000000..62556945159 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-42.c @@ -0,0 +1,19 @@ +/* { dg-do compile } */ +/* { dg-options "-O1 -fdump-tree-optimized" } */ + +/* PR tree-optimization/116890 */ + +int f(int a, int b, int c) +{ + int x; + if (c) x = a == 0; + else x = 0; + return x; +} + + +/* The if should have been removed as the the conversion from bool to int should have been factored out. */ +/* { dg-final { scan-tree-dump-not "if" "optimized" } }*/ +/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = a_\[0-9\]*.D. == 0" 1 "optimized" } } */ +/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = c_\[0-9\]*.D. != 0" 1 "optimized" } } */ + diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-43.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-43.c new file mode 100644 index 00000000000..1d16f283f27 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-43.c @@ -0,0 +1,19 @@ +/* { dg-do compile } */ +/* { dg-options "-O1 -fdump-tree-optimized" } */ + +/* PR tree-optimization/116890 */ + +int f(_Bool a, _Bool b, int c) +{ + int x; + if (c) x = a & b; + else x = 0; + return x; +} + + +/* The if should have been removed as the the conversion from bool to int should have been factored out. */ +/* { dg-final { scan-tree-dump-not "if" "optimized" } }*/ +/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = a_\[0-9\]*.D. & b_\[0-9\]*.D." 1 "optimized" } } */ +/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = c_\[0-9\]*.D. != 0" 1 "optimized" } } */ + diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-7.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-7.c index 51e1f6dfa75..3ee43e55692 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-7.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-7.c @@ -15,8 +15,8 @@ int f(int t, int c) return g(d,e); } -/* There should be one ifs as one of them should be changed into - a conditional and the other should be there still. */ -/* { dg-final { scan-tree-dump-times "if" 1 "optimized" } }*/ +/* There should be no ifs as this is converted into `(t != 0) & (c != 0)`. +/* { dg-final { scan-tree-dump-not "if" "optimized" } }*/ /* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = c_\[0-9\]*.D. != 0" 1 "optimized" } } */ +/* { dg-final { scan-tree-dump-times "\[^\r\n\]*_\[0-9\]* = t_\[0-9\]*.D. != 0" 1 "optimized" } } */ diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc index d43832b390b..bd7f9607eb9 100644 --- a/gcc/tree-ssa-phiopt.cc +++ b/gcc/tree-ssa-phiopt.cc @@ -345,9 +345,17 @@ factor_out_conditional_operation (edge e0, edge e1, gphi *phi, if (gassign *assign = dyn_cast <gassign *> (stmt)) { tree lhs = gimple_assign_lhs (assign); + tree lhst = TREE_TYPE (lhs); enum tree_code ass_code = gimple_assign_rhs_code (assign); - if (ass_code != MAX_EXPR && ass_code != MIN_EXPR) + if (ass_code != MAX_EXPR && ass_code != MIN_EXPR + /* Conversions from boolean like types is ok + as `a?1:b` and `a?0:b` will always simplify + to `a & b` or `a | b`. + See PR 116890. */ + && !(INTEGRAL_TYPE_P (lhst) + && TYPE_UNSIGNED (lhst) + && TYPE_PRECISION (lhst) == 1)) return NULL; if (lhs != gimple_assign_rhs1 (arg0_def_stmt)) return NULL;
Take: ``` if (t_3(D) != 0) goto <bb 3>; else goto <bb 4>; <bb 3> _8 = c_4(D) != 0; _9 = (int) _8; <bb 4> # e_2 = PHI <_9(3), 0(2)> ``` We should factor out the conversion here as that will allow a simplfication to `(t_3 != 0) & (c_4 != 0)`. Unlike most other types; `a ? b : CST` will simplify for boolean result type to either `a | b` or `a & b` so allowing this conversion for all operations will be always profitable. Bootstrapped and tested on x86_64-linux-gnu with no regressions. Note on the phi-opt-7.c testcase change, we are now able to optimize this and remove the if due to the factoring out now so this is an improvement. PR tree-optimization/116890 gcc/ChangeLog: * tree-ssa-phiopt.cc (factor_out_conditional_operation): Conversions from bool is also should be considered as wanting to happen. gcc/testsuite/ChangeLog: * gcc.dg/tree-ssa/phi-opt-7.c: Update testcase for no ifs left. * gcc.dg/tree-ssa/phi-opt-42.c: New test. * gcc.dg/tree-ssa/phi-opt-43.c: New test. Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> --- gcc/testsuite/gcc.dg/tree-ssa/phi-opt-42.c | 19 +++++++++++++++++++ gcc/testsuite/gcc.dg/tree-ssa/phi-opt-43.c | 19 +++++++++++++++++++ gcc/testsuite/gcc.dg/tree-ssa/phi-opt-7.c | 6 +++--- gcc/tree-ssa-phiopt.cc | 10 +++++++++- 4 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-42.c create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-43.c