Fix PR56426

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 :(.

Regtested/bootstrapped on x86_64-linux, ok for trunk?

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.


	Marek

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.