| Message ID | alpine.DEB.2.02.1406041254330.7525@stedding.saclay.inria.fr |
|---|---|
| State | New |
| Headers | show |
On Wed, Jun 4, 2014 at 1:20 PM, Marc Glisse <marc.glisse@inria.fr> wrote: > On Wed, 4 Jun 2014, Richard Biener wrote: > >> So I'd say we should instead simply bail out if the middle-bb has a PHI >> node. > > > Sounds good to me, so I am testing the mini-patch I had originally posted to > bugzilla: Ok. Thanks, Richard. > > 2014-06-04 Marc Glisse <marc.glisse@inria.fr> > > PR tree-optimization/61385 > gcc/ > * tree-ssa-phiopt.c (value_replacement): Punt if there are PHI > nodes. > > gcc/testsuite/ > * gcc.dg/tree-ssa/pr61385.c: New file. > > -- > Marc Glisse > Index: gcc/testsuite/gcc.dg/tree-ssa/pr61385.c > =================================================================== > --- gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (revision 0) > +++ gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (working copy) > @@ -0,0 +1,43 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2" } */ > + > +#define assert(x) if (!(x)) __builtin_abort () > + > +int a, b, c, d, e, f, g; > + > +int > +fn1 () > +{ > + int *h = &c; > + for (; c < 1; c++) > + { > + int *i = &a, *k = &a; > + f = 0; > + if (b) > + return 0; > + if (*h) > + { > + int **j = &i; > + *j = 0; > + d = 0; > + } > + else > + g = e = 0; > + if (*h) > + { > + int **l = &k; > + *l = &g; > + } > + d &= *h; > + assert (k == &a || k); > + assert (i); > + } > + return 0; > +} > + > +int > +main () > +{ > + fn1 (); > + return 0; > +} > Index: gcc/tree-ssa-phiopt.c > =================================================================== > --- gcc/tree-ssa-phiopt.c (revision 211221) > +++ gcc/tree-ssa-phiopt.c (working copy) > @@ -842,20 +842,24 @@ value_replacement (basic_block cond_bb, > /* Now optimize (x != 0) ? x + y : y to just y. > The following condition is too restrictive, there can easily be > another > stmt in middle_bb, for instance a CONVERT_EXPR for the second > argument. */ > gimple assign = last_and_only_stmt (middle_bb); > if (!assign || gimple_code (assign) != GIMPLE_ASSIGN > || gimple_assign_rhs_class (assign) != GIMPLE_BINARY_RHS > || (!INTEGRAL_TYPE_P (TREE_TYPE (arg0)) > && !POINTER_TYPE_P (TREE_TYPE (arg0)))) > return 0; > > + /* Punt if there are (degenerate) PHIs in middle_bb, there should not be. > */ > + if (!gimple_seq_empty_p (phi_nodes (middle_bb))) > + return 0; > + > /* Only transform if it removes the condition. */ > if (!single_non_singleton_phi_for_edges (phi_nodes (gimple_bb (phi)), e0, > e1)) > return 0; > > /* Size-wise, this is always profitable. */ > if (optimize_bb_for_speed_p (cond_bb) > /* The special case is useless if it has a low probability. */ > && profile_status_for_fn (cfun) != PROFILE_ABSENT > && EDGE_PRED (middle_bb, 0)->probability < PROB_EVEN > /* If assign is cheap, there is no point avoiding it. */ >
Index: gcc/testsuite/gcc.dg/tree-ssa/pr61385.c =================================================================== --- gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (revision 0) +++ gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (working copy) @@ -0,0 +1,43 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +#define assert(x) if (!(x)) __builtin_abort () + +int a, b, c, d, e, f, g; + +int +fn1 () +{ + int *h = &c; + for (; c < 1; c++) + { + int *i = &a, *k = &a; + f = 0; + if (b) + return 0; + if (*h) + { + int **j = &i; + *j = 0; + d = 0; + } + else + g = e = 0; + if (*h) + { + int **l = &k; + *l = &g; + } + d &= *h; + assert (k == &a || k); + assert (i); + } + return 0; +} + +int +main () +{ + fn1 (); + return 0; +} Index: gcc/tree-ssa-phiopt.c =================================================================== --- gcc/tree-ssa-phiopt.c (revision 211221) +++ gcc/tree-ssa-phiopt.c (working copy) @@ -842,20 +842,24 @@ value_replacement (basic_block cond_bb, /* Now optimize (x != 0) ? x + y : y to just y. The following condition is too restrictive, there can easily be another stmt in middle_bb, for instance a CONVERT_EXPR for the second argument. */ gimple assign = last_and_only_stmt (middle_bb); if (!assign || gimple_code (assign) != GIMPLE_ASSIGN || gimple_assign_rhs_class (assign) != GIMPLE_BINARY_RHS || (!INTEGRAL_TYPE_P (TREE_TYPE (arg0)) && !POINTER_TYPE_P (TREE_TYPE (arg0)))) return 0; + /* Punt if there are (degenerate) PHIs in middle_bb, there should not be. */ + if (!gimple_seq_empty_p (phi_nodes (middle_bb))) + return 0; + /* Only transform if it removes the condition. */ if (!single_non_singleton_phi_for_edges (phi_nodes (gimple_bb (phi)), e0, e1)) return 0; /* Size-wise, this is always profitable. */ if (optimize_bb_for_speed_p (cond_bb) /* The special case is useless if it has a low probability. */ && profile_status_for_fn (cfun) != PROFILE_ABSENT && EDGE_PRED (middle_bb, 0)->probability < PROB_EVEN /* If assign is cheap, there is no point avoiding it. */
On Wed, 4 Jun 2014, Richard Biener wrote: > So I'd say we should instead simply bail out if the middle-bb has a PHI node. Sounds good to me, so I am testing the mini-patch I had originally posted to bugzilla: 2014-06-04 Marc Glisse <marc.glisse@inria.fr> PR tree-optimization/61385 gcc/ * tree-ssa-phiopt.c (value_replacement): Punt if there are PHI nodes. gcc/testsuite/ * gcc.dg/tree-ssa/pr61385.c: New file.