This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch] Fix PR43464: update loop closed SSA form once copy prop is done
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.
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