Message ID | 20130225195311.GB25197@redhat.com |
---|---|
State | New |
Headers | show |
On Mon, Feb 25, 2013 at 8:53 PM, Marek Polacek wrote: > This fixes PR56426. We were ICEing during the loop pipeline, > because copyprop changed an irreducible region into a reducible - thus > the number_of_loops grew. We've seen that kind of thing happen before with the tracer pass. It makes me worried a bit about the robustness of preserving loops... Is there anything in the loop maintenance frame work that catches and handles new loops if they are "spontaneously" created from previously irreducible regions? Ciao! Steven
On Mon, Feb 25, 2013 at 09:01:17PM +0100, Steven Bosscher wrote: > On Mon, Feb 25, 2013 at 8:53 PM, Marek Polacek wrote: > > This fixes PR56426. We were ICEing during the loop pipeline, > > because copyprop changed an irreducible region into a reducible - thus > > the number_of_loops grew. > > We've seen that kind of thing happen before with the tracer pass. It > makes me worried a bit about the robustness of preserving loops... > > Is there anything in the loop maintenance frame work that catches and > handles new loops if they are "spontaneously" created from previously > irreducible regions? Well - flow_loops_find now works incrementally on an existing loop tree, so when the new loops appear, this function is able to detect them. When passing the NULL parameter, loops are computed from scratch. It is what e.g. fix_loop_structure now uses. Marek
On Mon, 25 Feb 2013, Marek Polacek wrote: > This fixes PR56426. We were ICEing during the loop pipeline, > because copyprop changed an irreducible region into a reducible - thus > the number_of_loops grew. Firstly, in tree_ssa_loop_init, number_of_loops > was == 1, which means we didn't initialize SCEV. But then later on, in > tree_ssa_loop_bounds, the number_of_loops was 2, thus we called > estimate_numbers_of_iterations. Oops. So fixed by always initializing > SCEV in the loop initialization on the tree level. > > Another approach would be to skip the whole loop pipeline - but then > we'd miss e.g. the BB vectorization. > > Richi: no, I didn't forget to change the TODO_update_ssa to 0, but that > ICEd pretty soon when building gcc :(. Eh ... ;) Ok, just forget that part then (update_ssa asserts it get's a valid update flag and rewrite_into_loop_closed_ssa calls update_ssa unconditionally) - it won't do anything if !need_ssa_update_p anyway, just at this point it really should never do anything. > Regtested/bootstrapped on x86_64-linux, ok for trunk? Ok. Thanks, Richard. > 2013-02-25 Marek Polacek <polacek@redhat.com> > > PR tree-optimization/56426 > * tree-ssa-loop.c (tree_ssa_loop_init): Always call > scev_initialize. > > * gcc.dg/pr56436.c: New test. > > --- gcc/tree-ssa-loop.c.mp 2013-02-25 13:06:47.212132327 +0100 > +++ gcc/tree-ssa-loop.c 2013-02-25 20:09:30.668978936 +0100 > @@ -70,10 +70,13 @@ tree_ssa_loop_init (void) > | LOOPS_HAVE_RECORDED_EXITS); > rewrite_into_loop_closed_ssa (NULL, TODO_update_ssa); > > + /* We might discover new loops, e.g. when turning irreducible > + regions into reducible. */ > + scev_initialize (); > + > if (number_of_loops () <= 1) > return 0; > > - scev_initialize (); > return 0; > } > > --- gcc/testsuite/gcc.dg/pr56436.c.mp 2013-02-25 20:19:10.213561026 +0100 > +++ gcc/testsuite/gcc.dg/pr56436.c 2013-02-25 20:19:02.944541180 +0100 > @@ -0,0 +1,22 @@ > +/* PR tree-optimization/56426 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2" } */ > + > +int a, *c; > + > +void > +f (void) > +{ > + int b = 0; > + > + for (a = 0;; a++) > + if (--b) > + { > + if (a) > + lbl: > + a++; > + > + c = &b; > + goto lbl; > + } > +} > > Marek > >
On Mon, 25 Feb 2013, Steven Bosscher wrote: > On Mon, Feb 25, 2013 at 8:53 PM, Marek Polacek wrote: > > This fixes PR56426. We were ICEing during the loop pipeline, > > because copyprop changed an irreducible region into a reducible - thus > > the number_of_loops grew. > > We've seen that kind of thing happen before with the tracer pass. It > makes me worried a bit about the robustness of preserving loops... > > Is there anything in the loop maintenance frame work that catches and > handles new loops if they are "spontaneously" created from previously > irreducible regions? Yes, I very recently have re-written fix_loop_structure to take care of discovering new loops (via making flow_loops_find work incremental ontop of an existing loop tree). The only drawback is that it is now too easy to not "preserve" loops (that is, keep their number and struct loop the same) by first dropping it and then re-discover it via fix_loop_structure. At least dump files now contain both loop nodes that are removed and loop nodes that are newly discovered. Richard.
--- gcc/tree-ssa-loop.c.mp 2013-02-25 13:06:47.212132327 +0100 +++ gcc/tree-ssa-loop.c 2013-02-25 20:09:30.668978936 +0100 @@ -70,10 +70,13 @@ tree_ssa_loop_init (void) | LOOPS_HAVE_RECORDED_EXITS); rewrite_into_loop_closed_ssa (NULL, TODO_update_ssa); + /* We might discover new loops, e.g. when turning irreducible + regions into reducible. */ + scev_initialize (); + if (number_of_loops () <= 1) return 0; - scev_initialize (); return 0; } --- gcc/testsuite/gcc.dg/pr56436.c.mp 2013-02-25 20:19:10.213561026 +0100 +++ gcc/testsuite/gcc.dg/pr56436.c 2013-02-25 20:19:02.944541180 +0100 @@ -0,0 +1,22 @@ +/* PR tree-optimization/56426 */ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +int a, *c; + +void +f (void) +{ + int b = 0; + + for (a = 0;; a++) + if (--b) + { + if (a) + lbl: + a++; + + c = &b; + goto lbl; + } +}