[patch] Fix PR43464: update loop closed SSA form once copy prop is done

[Jack Howarth]

On Fri, Mar 26, 2010 at 10:51:12AM +0100, Richard Guenther wrote:
> On Wed, 24 Mar 2010, Sebastian Pop wrote:
> 
> > On Tue, Mar 23, 2010 at 22:40, Kenneth Zadeck
> > <Kenneth.Zadeck@naturalbridge.com> wrote:
> > > Let me try to bring some perspective back into this discussion,
> > > because it is clear that this is getting out of hand.
> > >
> > > There are two solutions that must be considered: a short term one and
> > > a long term one.
> > >
> > > At this stage, a rewrite of a pass of the compiler is not acceptable,
> > > so calling that rewrite_into_closed_loop_ssa form in a place
> > > immediately after the pass that messed it up is the correct short term
> > > solution.  And it is purely Richard's call as to how careful that
> > > patch needs to be about checking if things get messed up.
> > >
> > 
> > Richi, would the patch below be ok for 4.5?
> > This passes bootstrap and test on amd64-linux,
> > and the SPEC 2006 benchmarks with the all the
> > graphite transforms enabled.
> > 
> > I am proposing to keep the PR43464 open and mark it
> > high priority for 4.6.
> > 
> > Thanks,
> > Sebastian
> > 
> > ----------------------------- gcc/tree-ssa-copy.c -----------------------------
> > index 4b8d0b9..683ea64 100644
> > @@ -968,6 +968,21 @@ execute_copy_prop (void)
> >    init_copy_prop ();
> >    ssa_propagate (copy_prop_visit_stmt, copy_prop_visit_phi_node);
> >    fini_copy_prop ();
> > +
> > +  /* FIXME: The call to rewrite_into_loop_closed_ssa should be
> > +     removed: this code is a temporary work around needed to maintain
> > +     the loop closed SSA form when copy propagation is called after
> > +     loop unswitching pass.  The second call to copy propagation is
> > +     done as a sub-pass of the graphite pass, and that's why the
> > +     flag_graphite is tested here.  */
> > +  if (current_loops && flag_graphite)
> 
> But we already analyzed that copyprop is _not_ the problem.  The
> problem is the pass that introduces the two-arg loop-closed PHI
> node.
> 
> Anyway, the condition should be current_loops && loops_state_satisfies_p 
> (LOOP_CLOSED_SSA).  And the checking is definitely not useful.
> 

So are you saying that...

Index: gcc/tree-ssa-copy.c
===================================================================
--- gcc/tree-ssa-copy.c	(revision 157746)
+++ gcc/tree-ssa-copy.c	(working copy)
@@ -968,6 +968,18 @@
   init_copy_prop ();
   ssa_propagate (copy_prop_visit_stmt, copy_prop_visit_phi_node);
   fini_copy_prop ();
+
+ /* FIXME: The call to rewrite_into_loop_closed_ssa should be
+    removed: this code is a temporary work around needed to maintain
+    the loop closed SSA form when copy propagation is called after
+    loop unswitching pass.  The second call to copy propagation is
+    done as a sub-pass of the graphite pass, and that's why the
+    flag_graphite is tested here.  */
+ if (current_loops && loops_state_satisfies_p(LOOP_CLOSED_SSA) && flag_graphite)
+   {
+     rewrite_into_loop_closed_ssa (NULL, TODO_update_ssa);
+   }
+
   return 0;
 }
 
Index: gcc/passes.c
===================================================================
--- gcc/passes.c	(revision 157746)
+++ gcc/passes.c	(working copy)
@@ -897,6 +897,7 @@
 	  NEXT_PASS (pass_graphite_transforms);
 	    {
 	      struct opt_pass **p = &pass_graphite_transforms.pass.sub;
+	      NEXT_PASS (pass_copy_prop);
 	      NEXT_PASS (pass_dce_loop);
 	      NEXT_PASS (pass_lim);
 	    }

would be okay?
           Jack

> The patch is not ok.
> 
> Thanks,
> Richard.
> 
> > +    {
> > +      rewrite_into_loop_closed_ssa (NULL, TODO_update_ssa);
> > +#ifdef ENABLE_CHECKING
> > +      verify_loop_closed_ssa ();
> > +#endif
> > +    }
> > +
> >    return 0;
> >  }
> > 
> > --------------------------------- gcc/passes.c ---------------------------------
> > index 9d0c55a..1ac8694 100644
> > @@ -897,6 +897,7 @@ init_optimization_passes (void)
> >  	  NEXT_PASS (pass_graphite_transforms);
> >  	    {
> >  	      struct opt_pass **p = &pass_graphite_transforms.pass.sub;
> > +	      NEXT_PASS (pass_copy_prop);
> >  	      NEXT_PASS (pass_dce_loop);
> >  	      NEXT_PASS (pass_lim);
> >  	    }
> > 
> > 
> 
> -- 
> Richard Guenther <rguenther@suse.de>
> Novell / SUSE Labs
> SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex