| Message ID | 20241012014123.4088037-1-quic_apinski@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | phiopt: Clobbers can sometimes get in the way of phiopt [PR117096] | expand |
On Sat, Oct 12, 2024 at 3:42 AM Andrew Pinski <quic_apinski@quicinc.com> wrote: > > Clobbers in a condition don't cause any improvements and are getting > in the way of doing match and simplify with phiopt. Need to ignore/skip > over them in when seeing if there is only one statement that can moved. > Also since clobbers have vops, skipping over the vops phi node is needed > to be done. But you can't necessarily move things across a clobber, do all users of those functions then remove the clobber? > Bootstrapped and tested on x86_64-linux-gnu. > > PR tree-optimization/117096 > > gcc/ChangeLog: > > * tree-ssa-phiopt.cc (single_non_singleton_phi_for_edges): Skip > over vops rather than return false. > (empty_bb_or_one_feeding_into_p): Skip over clobbers too. > > gcc/testsuite/ChangeLog: > > * g++.dg/tree-ssa/phiopt-2.C: New test. > > Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> > --- > gcc/testsuite/g++.dg/tree-ssa/phiopt-2.C | 46 ++++++++++++++++++++++++ > gcc/tree-ssa-phiopt.cc | 14 ++++---- > 2 files changed, 54 insertions(+), 6 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/tree-ssa/phiopt-2.C > > diff --git a/gcc/testsuite/g++.dg/tree-ssa/phiopt-2.C b/gcc/testsuite/g++.dg/tree-ssa/phiopt-2.C > new file mode 100644 > index 00000000000..9bc8fb6e8dc > --- /dev/null > +++ b/gcc/testsuite/g++.dg/tree-ssa/phiopt-2.C > @@ -0,0 +1,46 @@ > +// { dg-do compile } > +// { dg-options "-O1 -fdump-tree-phiopt1-details" } > +// PR tree-optimization/117096 > +// Clobbers should NOT make a difference > +// when it comes to phiopt > + > +struct s1{ > + unsigned b; > + > + // Declare as always inline just in some someone turns down the inlining limits > + // we also want to inline early. > + __attribute__((always_inline)) inline > + s1() : b(0) {} > +}; > +void g(); > +int f(signed a, int c) > +{ > + signed b = 0; > + if (a < 0) > + { > + s1(); > + b = a; > + } > + else { s1(); } > + g(); > + return b; > +} > + > + > +int f1(signed a, int c) > +{ > + signed b = 0; > + if (a < 0) > + { > + s1 t; > + b = a; > + } > + else { s1 t; } > + g(); > + return b; > +} > + > +/* both if should be converted into MIN_EXPR */ > +/* { dg-final { scan-tree-dump-not "if " "phiopt1" } }*/ > +/* { dg-final { scan-tree-dump-times "MIN_EXPR " 2 "phiopt1" } }*/ > + > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc > index f3ee3a80c0f..b22086f4de8 100644 > --- a/gcc/tree-ssa-phiopt.cc > +++ b/gcc/tree-ssa-phiopt.cc > @@ -72,9 +72,10 @@ single_non_singleton_phi_for_edges (gimple_seq seq, edge e0, edge e1) > gimple_phi_arg_def (p, e1->dest_idx))) > continue; > > - /* Punt on virtual phis with different arguments from the edges. */ > + /* Skip virtual phis with different arguments > + from the edges as clobbers might cause the vop to be different. */ > if (virtual_operand_p (gimple_phi_result (p))) > - return NULL; > + continue; > > /* If we already have a PHI that has the two edge arguments are > different, then return it is not a singleton for these PHIs. */ > @@ -663,9 +664,10 @@ empty_bb_or_one_feeding_into_p (basic_block bb, > { > gimple *s = gsi_stmt (gsi); > gsi_next_nondebug (&gsi); > - /* Skip over Predict and nop statements. */ > + /* Skip over Predict, clobber and nop statements. */ > if (gimple_code (s) == GIMPLE_PREDICT > - || gimple_code (s) == GIMPLE_NOP) > + || gimple_code (s) == GIMPLE_NOP > + || gimple_clobber_p (s)) > continue; > /* If there is more one statement return false. */ > if (stmt_to_move) > @@ -673,8 +675,8 @@ empty_bb_or_one_feeding_into_p (basic_block bb, > stmt_to_move = s; > } > > - /* The only statement here was a Predict or a nop statement > - so return true. */ > + /* The only statement here was a Predict, nop, > + or a clobber statement so return true. */ > if (!stmt_to_move) > return true; > > -- > 2.43.0 >
diff --git a/gcc/testsuite/g++.dg/tree-ssa/phiopt-2.C b/gcc/testsuite/g++.dg/tree-ssa/phiopt-2.C new file mode 100644 index 00000000000..9bc8fb6e8dc --- /dev/null +++ b/gcc/testsuite/g++.dg/tree-ssa/phiopt-2.C @@ -0,0 +1,46 @@ +// { dg-do compile } +// { dg-options "-O1 -fdump-tree-phiopt1-details" } +// PR tree-optimization/117096 +// Clobbers should NOT make a difference +// when it comes to phiopt + +struct s1{ + unsigned b; + + // Declare as always inline just in some someone turns down the inlining limits + // we also want to inline early. + __attribute__((always_inline)) inline + s1() : b(0) {} +}; +void g(); +int f(signed a, int c) +{ + signed b = 0; + if (a < 0) + { + s1(); + b = a; + } + else { s1(); } + g(); + return b; +} + + +int f1(signed a, int c) +{ + signed b = 0; + if (a < 0) + { + s1 t; + b = a; + } + else { s1 t; } + g(); + return b; +} + +/* both if should be converted into MIN_EXPR */ +/* { dg-final { scan-tree-dump-not "if " "phiopt1" } }*/ +/* { dg-final { scan-tree-dump-times "MIN_EXPR " 2 "phiopt1" } }*/ + diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc index f3ee3a80c0f..b22086f4de8 100644 --- a/gcc/tree-ssa-phiopt.cc +++ b/gcc/tree-ssa-phiopt.cc @@ -72,9 +72,10 @@ single_non_singleton_phi_for_edges (gimple_seq seq, edge e0, edge e1) gimple_phi_arg_def (p, e1->dest_idx))) continue; - /* Punt on virtual phis with different arguments from the edges. */ + /* Skip virtual phis with different arguments + from the edges as clobbers might cause the vop to be different. */ if (virtual_operand_p (gimple_phi_result (p))) - return NULL; + continue; /* If we already have a PHI that has the two edge arguments are different, then return it is not a singleton for these PHIs. */ @@ -663,9 +664,10 @@ empty_bb_or_one_feeding_into_p (basic_block bb, { gimple *s = gsi_stmt (gsi); gsi_next_nondebug (&gsi); - /* Skip over Predict and nop statements. */ + /* Skip over Predict, clobber and nop statements. */ if (gimple_code (s) == GIMPLE_PREDICT - || gimple_code (s) == GIMPLE_NOP) + || gimple_code (s) == GIMPLE_NOP + || gimple_clobber_p (s)) continue; /* If there is more one statement return false. */ if (stmt_to_move) @@ -673,8 +675,8 @@ empty_bb_or_one_feeding_into_p (basic_block bb, stmt_to_move = s; } - /* The only statement here was a Predict or a nop statement - so return true. */ + /* The only statement here was a Predict, nop, + or a clobber statement so return true. */ if (!stmt_to_move) return true;
Clobbers in a condition don't cause any improvements and are getting in the way of doing match and simplify with phiopt. Need to ignore/skip over them in when seeing if there is only one statement that can moved. Also since clobbers have vops, skipping over the vops phi node is needed to be done. Bootstrapped and tested on x86_64-linux-gnu. PR tree-optimization/117096 gcc/ChangeLog: * tree-ssa-phiopt.cc (single_non_singleton_phi_for_edges): Skip over vops rather than return false. (empty_bb_or_one_feeding_into_p): Skip over clobbers too. gcc/testsuite/ChangeLog: * g++.dg/tree-ssa/phiopt-2.C: New test. Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> --- gcc/testsuite/g++.dg/tree-ssa/phiopt-2.C | 46 ++++++++++++++++++++++++ gcc/tree-ssa-phiopt.cc | 14 ++++---- 2 files changed, 54 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/g++.dg/tree-ssa/phiopt-2.C