This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Fri, Mar 26, 2010 at 04:51, Richard Guenther <rguenther@suse.de> wrote:
> But we already analyzed that copyprop is _not_ the problem. ?The
> problem is the pass that introduces the two-arg loop-closed PHI
> node.

Note that pass_tree_unswitch is not the only pass that introduces
the two-arg loop-closed PHI.  Copyprop is another pass that does that,
as it has a TODO_cleanup_cfg that calls cleanup_tree_cfg.

cleanup_tree_cfg_noloop called from cleanup_tree_cfg is destroying the
CFG structure needed for the LCSSA form: it removes all the single entry
basic blocks that were placed after the loops for the LCSSA form.

Interestingly, cleanup_tree_cfg is calling repair_loop_structures

  if (current_loops != NULL
      && loops_state_satisfies_p (LOOPS_NEED_FIXUP))
    repair_loop_structures ();

that in turn calls rewrite_into_loop_closed_ssa like this:

  /* This usually does nothing.  But sometimes parts of cfg that originally
     were inside a loop get out of it due to edge removal (since they
     become unreachable by back edges from latch).  */
  if (loops_state_satisfies_p (LOOP_CLOSED_SSA))
    rewrite_into_loop_closed_ssa (changed_bbs, TODO_update_ssa);

and this call to rewrite_into_loop_closed_ssa does not add the single
entry basic block needed to have a proper LCSSA form.

Should rewrite_into_loop_closed_ssa replace the missing basic blocks,
or should cleanup_tree_cfg_noloop be fixed to not destroy the CFG
form needed for the LCSSA?

Sebastian

PS: to reproduce the errors described here you would need the attached
patches.
From 7f9d583a500c2f7a9039a68ad078916ddb16cda3 Mon Sep 17 00:00:00 2001
From: Sebastian Pop <sebpop@gmail.com>
Date: Sun, 28 Mar 2010 13:29:04 -0500
Subject: [PATCH 1/2] Enforce 1 argument loop close phi nodes in verify_loop_closed_ssa.

---
 gcc/tree-ssa-loop-manip.c |   21 +++++++++++++++++++--
 1 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/gcc/tree-ssa-loop-manip.c b/gcc/tree-ssa-loop-manip.c
index 7818f5b..cf6c541 100644
--- a/gcc/tree-ssa-loop-manip.c
+++ b/gcc/tree-ssa-loop-manip.c
@@ -424,6 +424,24 @@ check_loop_closed_ssa_use (basic_block bb, tree use)
 	      || flow_bb_inside_loop_p (def_bb->loop_father, bb));
 }
 
+/* Check the invariants of the loop close SSA form for the PHI node,
+   and the phi argument coming from edge E.  */
+
+static void
+check_loop_closed_ssa_phi (gimple phi, edge e)
+{
+  tree use = PHI_ARG_DEF_FROM_EDGE (phi, e);
+
+  if (TREE_CODE (use) != SSA_NAME || !is_gimple_reg (use))
+    return;
+
+  /* Loop close phi nodes should have exactly 1 argument.  */
+  gcc_assert (flow_bb_inside_loop_p (e->src->loop_father, gimple_bb (phi))
+	      || gimple_phi_num_args (phi) == 1);
+
+  check_loop_closed_ssa_use (e->src, use);
+}
+
 /* Checks invariants of loop closed ssa form in statement STMT in BB.  */
 
 static void
@@ -461,8 +479,7 @@ verify_loop_closed_ssa (void)
 	{
 	  phi = gsi_stmt (bsi);
 	  FOR_EACH_EDGE (e, ei, bb->preds)
-	    check_loop_closed_ssa_use (e->src,
-				       PHI_ARG_DEF_FROM_EDGE (phi, e));
+	    check_loop_closed_ssa_phi (phi, e);
 	}
 
       for (bsi = gsi_start_bb (bb); !gsi_end_p (bsi); gsi_next (&bsi))
-- 
1.6.3.3

From 1936d582a28f0c72aef6c8d661b9cdc5b3e6d705 Mon Sep 17 00:00:00 2001
From: Sebastian Pop <sebpop@gmail.com>
Date: Mon, 29 Mar 2010 10:25:26 -0500
Subject: [PATCH 2/2] Add TODO_verify_loops for all the passes called from LNO.

---
 gcc/tree-ssa-copy.c     |    1 +
 gcc/tree-ssa-dce.c      |    4 +++-
 gcc/tree-ssa-loop.c     |    2 ++
 gcc/tree-vect-generic.c |    1 +
 gcc/tree-vectorizer.c   |    1 +
 5 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/gcc/tree-ssa-copy.c b/gcc/tree-ssa-copy.c
index 4b8d0b9..356da1e 100644
--- a/gcc/tree-ssa-copy.c
+++ b/gcc/tree-ssa-copy.c
@@ -993,6 +993,7 @@ struct gimple_opt_pass pass_copy_prop =
   0,					/* properties_destroyed */
   0,					/* todo_flags_start */
   TODO_cleanup_cfg
+    | TODO_verify_loops
     | TODO_dump_func
     | TODO_ggc_collect
     | TODO_verify_ssa
diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
index 4e3499a..83deaa9 100644
--- a/gcc/tree-ssa-dce.c
+++ b/gcc/tree-ssa-dce.c
@@ -1450,7 +1450,9 @@ struct gimple_opt_pass pass_dce_loop =
   0,					/* properties_provided */
   0,					/* properties_destroyed */
   0,					/* todo_flags_start */
-  TODO_dump_func | TODO_verify_ssa	/* todo_flags_finish */
+  TODO_dump_func
+    | TODO_verify_loops
+    | TODO_verify_ssa	/* todo_flags_finish */
  }
 };
 
diff --git a/gcc/tree-ssa-loop.c b/gcc/tree-ssa-loop.c
index 591890c..37c79f5 100644
--- a/gcc/tree-ssa-loop.c
+++ b/gcc/tree-ssa-loop.c
@@ -246,6 +246,7 @@ struct gimple_opt_pass pass_vectorize =
   0,                                    /* properties_destroyed */
   TODO_verify_loops,			/* todo_flags_start */
   TODO_dump_func | TODO_update_ssa
+    | TODO_verify_loops
     | TODO_ggc_collect			/* todo_flags_finish */
  }
 };
@@ -431,6 +432,7 @@ struct gimple_opt_pass pass_scev_cprop =
   0,					/* todo_flags_start */
   TODO_dump_func | TODO_cleanup_cfg
     | TODO_update_ssa_only_virtuals
+    | TODO_verify_loops
 					/* todo_flags_finish */
  }
 };
diff --git a/gcc/tree-vect-generic.c b/gcc/tree-vect-generic.c
index 2bb71a1..d886b9d 100644
--- a/gcc/tree-vect-generic.c
+++ b/gcc/tree-vect-generic.c
@@ -584,6 +584,7 @@ struct gimple_opt_pass pass_lower_vector_ssa =
   0,					/* properties_destroyed */
   0,					/* todo_flags_start */
   TODO_dump_func | TODO_update_ssa	/* todo_flags_finish */
+    | TODO_verify_loops
     | TODO_verify_ssa
     | TODO_verify_stmts | TODO_verify_flow
  }
diff --git a/gcc/tree-vectorizer.c b/gcc/tree-vectorizer.c
index e337672..0f39c22 100644
--- a/gcc/tree-vectorizer.c
+++ b/gcc/tree-vectorizer.c
@@ -311,6 +311,7 @@ struct gimple_opt_pass pass_slp_vectorize =
   0,                                    /* properties_destroyed */
   0,                                    /* todo_flags_start */
   TODO_ggc_collect
+    | TODO_verify_loops
     | TODO_verify_ssa
     | TODO_dump_func
     | TODO_update_ssa
-- 
1.6.3.3


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]