[Bug middle-end/43464] copy prop breaks loop closed SSA form
--- Comment #16 from spop at gcc dot gnu dot org 2010-03-31 18:48 --- Fixed. -- spop at gcc dot gnu dot org changed: What|Removed |Added Status|NEW |RESOLVED Resolution||FIXED http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43464
[Bug middle-end/43464] copy prop breaks loop closed SSA form
--- Comment #15 from spop at gcc dot gnu dot org 2010-03-31 18:38 --- Subject: Bug 43464 Author: spop Date: Wed Mar 31 18:37:41 2010 New Revision: 157889 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=157889 Log: Fix PR43464: copyprop should maintain loop close phi nodes with multiple arguments. 2010-03-30 Richard Guenther Zdenek Dvorak Sebastian Pop PR middle-end/43464 * tree-ssa-copy.c (init_copy_prop): Handle loop close phi nodes with multiple arguments. (execute_copy_prop): Remove call to rewrite_into_loop_closed_ssa. Modified: trunk/gcc/ChangeLog.graphite trunk/gcc/tree-ssa-copy.c -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43464
[Bug middle-end/43464] copy prop breaks loop closed SSA form
--- Comment #14 from spop at gcc dot gnu dot org 2010-03-30 16:57 --- Subject: Bug 43464 Author: spop Date: Tue Mar 30 16:56:49 2010 New Revision: 157828 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=157828 Log: Fix PR43464: copyprop should maintain loop close phi nodes with multiple arguments. 2010-03-30 Richard Guenther Zdenek Dvorak Sebastian Pop PR middle-end/43464 * tree-ssa-copy.c (init_copy_prop): Handle loop close phi nodes with multiple arguments. (execute_copy_prop): Remove call to rewrite_into_loop_closed_ssa. Modified: branches/graphite/gcc/ChangeLog.graphite branches/graphite/gcc/tree-ssa-copy.c -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43464
[Bug middle-end/43464] copy prop breaks loop closed SSA form
--- Comment #13 from spop at gcc dot gnu dot org 2010-03-22 15:47 --- Note that * tree-ssa-copy.c (init_copy_prop): Loop closed phi nodes can contain more than one argument. breaks both 464.h264ref and 416.gamess with -O3. See for example http://groups.google.com/group/gcc-graphite-test/browse_thread/thread/f5b0d912e90b598f -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43464
[Bug middle-end/43464] copy prop breaks loop closed SSA form
--- Comment #12 from spop at gcc dot gnu dot org 2010-03-22 01:29 --- Subject: Bug 43464 Author: spop Date: Mon Mar 22 01:28:51 2010 New Revision: 157617 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=157617 Log: Fix PR43464: loop close phi nodes can contain more than one argument. 2010-03-21 Sebastian Pop Richard Guenther PR middle-end/43464 * tree-ssa-copy.c (init_copy_prop): Loop closed phi nodes can contain more than one argument. (execute_copy_prop): Revert the previous change: do not call rewrite_into_loop_closed_ssa. * gcc.dg/graphite/id-pr43464.c: Remove compile warning. * gcc.dg/graphite/id-pr43464-1.c: New. Added: branches/graphite/gcc/testsuite/gcc.dg/graphite/id-pr43464-1.c Modified: branches/graphite/gcc/ChangeLog.graphite branches/graphite/gcc/testsuite/gcc.dg/graphite/id-pr43464.c branches/graphite/gcc/tree-ssa-copy.c -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43464
[Bug middle-end/43464] copy prop breaks loop closed SSA form
--- Comment #11 from spop at gcc dot gnu dot org 2010-03-22 00:26 --- I would still like to see some extra checking after copyprop: would this extra check be ok to commit with the fix? diff --git a/gcc/tree-ssa-copy.c b/gcc/tree-ssa-copy.c index 61e32cc..011a80a 100644 --- a/gcc/tree-ssa-copy.c +++ b/gcc/tree-ssa-copy.c @@ -967,6 +967,13 @@ execute_copy_prop (void) init_copy_prop (); ssa_propagate (copy_prop_visit_stmt, copy_prop_visit_phi_node); fini_copy_prop (); + +#ifdef ENABLE_CHECKING + if (current_loops + && loops_state_satisfies_p (LOOP_CLOSED_SSA)) +verify_loop_closed_ssa (); +#endif + return 0; } -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43464
[Bug middle-end/43464] copy prop breaks loop closed SSA form
--- Comment #10 from spop at gcc dot gnu dot org 2010-03-22 00:17 --- Yes, this patch does fix the problem. diff --git a/gcc/tree-ssa-copy.c b/gcc/tree-ssa-copy.c index 57c6558..61e32cc 100644 --- a/gcc/tree-ssa-copy.c +++ b/gcc/tree-ssa-copy.c @@ -797,7 +797,6 @@ init_copy_prop (void) PHI nodes. Technically this is only needed for loop exit PHIs, but this is difficult to query. */ || (current_loops - && gimple_phi_num_args (phi) == 1 && loops_state_satisfies_p (LOOP_CLOSED_SSA))) prop_set_simulate_again (phi, false); else I will bootstrap and test this again. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43464
[Bug middle-end/43464] copy prop breaks loop closed SSA form
--- Comment #9 from rguenth at gcc dot gnu dot org 2010-03-21 23:07 --- (In reply to comment #8) > (In reply to comment #7) > > The problem is that copyprop does this change: > > > > @@ -25,7 +25,7 @@ > >: > > # .MEM_16 = PHI <.MEM_18(10), .MEM_20(11)> > > # s_66 = PHI > > -s_13 = s_66; > > +s_13 = s_1; > > goto (got_it); > > > >} > > > > then verify_loop_closed_ssa () complains about the fact that s_1 is > > defined in loop_1 and used outside loop_1 in a non close-phi node. > > Ah, copyprop avoids _single_ arg PHIs because that may break loop-closed > SSA form. Why does loop closed SSA form suddenly have double-arg > loop-closed PHI nodes? > > Indeed Sebastians patch is a non-suitable hammer. Copyprop already tries > to avoid breaking loop-closed SSA form. /* In loop-closed SSA form do not copy-propagate through PHI nodes. Technically this is only needed for loop exit PHIs, but this is difficult to query. */ || (current_loops && gimple_phi_num_args (phi) == 1 && loops_state_satisfies_p (LOOP_CLOSED_SSA))) Thus it seems that && gimple_phi_num_args (phi) == 1 should be removed. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43464
[Bug middle-end/43464] copy prop breaks loop closed SSA form
--- Comment #8 from rguenth at gcc dot gnu dot org 2010-03-21 23:05 --- (In reply to comment #7) > The problem is that copyprop does this change: > > @@ -25,7 +25,7 @@ >: > # .MEM_16 = PHI <.MEM_18(10), .MEM_20(11)> > # s_66 = PHI > -s_13 = s_66; > +s_13 = s_1; > goto (got_it); > >} > > then verify_loop_closed_ssa () complains about the fact that s_1 is > defined in loop_1 and used outside loop_1 in a non close-phi node. Ah, copyprop avoids _single_ arg PHIs because that may break loop-closed SSA form. Why does loop closed SSA form suddenly have double-arg loop-closed PHI nodes? Indeed Sebastians patch is a non-suitable hammer. Copyprop already tries to avoid breaking loop-closed SSA form. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43464
[Bug middle-end/43464] copy prop breaks loop closed SSA form
--- Comment #7 from spop at gcc dot gnu dot org 2010-03-21 17:17 --- The problem is that copyprop does this change: @@ -25,7 +25,7 @@ : # .MEM_16 = PHI <.MEM_18(10), .MEM_20(11)> # s_66 = PHI -s_13 = s_66; +s_13 = s_1; goto (got_it); } then verify_loop_closed_ssa () complains about the fact that s_1 is defined in loop_1 and used outside loop_1 in a non close-phi node. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43464
[Bug middle-end/43464] copy prop breaks loop closed SSA form
--- Comment #6 from spop at gcc dot gnu dot org 2010-03-21 17:01 --- Further reduced testcase: typedef struct regnode { char flags; } regnode; extern const unsigned char A[]; char *foo (regnode *c, char *s, int norun) { int uskip; while (s + (uskip = A[*s])) { if ((c->flags || bar (c)) && norun) goto got_it; s += uskip; } got_it: return s; } Needs an extra patch to trigger the ICE: diff --git a/gcc/passes.c b/gcc/passes.c index 1ac8694..620487f 100644 --- a/gcc/passes.c +++ b/gcc/passes.c @@ -894,6 +894,10 @@ init_optimization_passes (void) NEXT_PASS (pass_check_data_deps); NEXT_PASS (pass_loop_distribution); NEXT_PASS (pass_linear_transform); + NEXT_PASS (pass_copy_prop); + NEXT_PASS (pass_dce_loop); + NEXT_PASS (pass_lim); + NEXT_PASS (pass_graphite_transforms); { struct opt_pass **p = &pass_graphite_transforms.pass.sub; -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43464
[Bug middle-end/43464] copy prop breaks loop closed SSA form
--- Comment #5 from sebpop at gmail dot com 2010-03-21 16:08 --- Subject: Re: copy prop breaks loop closed SSA form On Sun, Mar 21, 2010 at 04:54, steven at gcc dot gnu dot org wrote: > Why such a big hammer? 'cause I don't want to add more bugs, and no I don't think compile time matters. > You should be able to figure out which copy props are > allowed and which should be disallowed in loop-closed SSA form. patches are welcome. > Is "if (current_loops)" the right test here? This will break if Zdenek's > patches to keep loops around throughout ever makes it to the trunk. please propose a better patch. Thanks, Sebastian -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43464
[Bug middle-end/43464] copy prop breaks loop closed SSA form
--- Comment #4 from steven at gcc dot gnu dot org 2010-03-21 09:54 --- Why such a big hammer? You should be able to figure out which copy props are allowed and which should be disallowed in loop-closed SSA form. Is "if (current_loops)" the right test here? This will break if Zdenek's patches to keep loops around throughout ever makes it to the trunk. -- steven at gcc dot gnu dot org changed: What|Removed |Added Status|UNCONFIRMED |NEW Ever Confirmed|0 |1 Last reconfirmed|-00-00 00:00:00 |2010-03-21 09:54:56 date|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43464
[Bug middle-end/43464] copy prop breaks loop closed SSA form
--- Comment #3 from spop at gcc dot gnu dot org 2010-03-21 07:32 --- Subject: Bug 43464 Author: spop Date: Sun Mar 21 07:32:43 2010 New Revision: 157602 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=157602 Log: Fix PR43464: update loop close SSA once copy prop is done. 2010-03-21 Sebastian Pop PR middle-end/43464 * tree-ssa-copy.c (execute_copy_prop): Call rewrite_into_loop_closed_ssa and verify_loop_closed_ssa when copy prop is executed in the LNO. * gcc.dg/graphite/id-pr43464.c: New. Added: branches/graphite/gcc/testsuite/gcc.dg/graphite/id-pr43464.c Modified: branches/graphite/gcc/ChangeLog.graphite branches/graphite/gcc/tree-ssa-copy.c -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43464
[Bug middle-end/43464] copy prop breaks loop closed SSA form
--- Comment #2 from spop at gcc dot gnu dot org 2010-03-21 07:06 --- Created an attachment (id=20149) --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20149&action=view) proposed fix The proposed fix is to recompute the loop closed SSA form after copy propagation. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43464
[Bug middle-end/43464] copy prop breaks loop closed SSA form
--- Comment #1 from spop at gcc dot gnu dot org 2010-03-21 07:01 --- Created an attachment (id=20148) --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20148&action=view) pr43464.i -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43464