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]

[PATCH] Fix PR 60866


Hello,

As described in the PR in more details, this ICE is about the scheduler not being able to initialize its data structures for the new unconditional jump created when redirecting an edge and simplifying control flow. It so happens that the new jump is the only unscheduled instruction being left so we need to pass some of its data from the old jump. The difficulty is to support only this particular situation to retain the existing checking code and to pass down the required information to the point where it is needed.

I assume that we are able to use default parameters as I haven't heard otherwise.

Bootstrapped and tested on x86-64, OK for trunk and branches? Branches should also be safe as we're fixing only very specific corner case (hey, we regressed four releases and nobody noticed until now).

Yours,
Andrey

gcc/
2014-05-14  Andrey Belevantsev  <abel@ispras.ru>

	PR rtl-optimization/60866
	* sel-sched-ir (sel_init_new_insn): New parameter old_seqno.
	Default it to -1.  Pass it down to init_simplejump_data.
	(init_simplejump_data): New parameter old_seqno.  Pass it down
	to get_seqno_for_a_jump.
	(get_seqno_for_a_jump): New parameter old_seqno.  Use it for
	initializing new jump seqno as a last resort.  Add comment.
	(sel_redirect_edge_and_branch): Save old seqno of the conditional
	jump and pass it down to sel_init_new_insn.
	(sel_redirect_edge_and_branch_force): Likewise.

testsuite/
2014-05-14  Andrey Belevantsev  <abel@ispras.ru>

	PR rtl-optimization/60866
	* gcc.dg/pr60866.c: New test.
diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
index 868083b..3cba326 100644
--- a/gcc/sel-sched-ir.c
+++ b/gcc/sel-sched-ir.c
@@ -162,7 +162,7 @@ static void create_initial_data_sets (basic_block);
 static void free_av_set (basic_block);
 static void invalidate_av_set (basic_block);
 static void extend_insn_data (void);
-static void sel_init_new_insn (insn_t, int);
+static void sel_init_new_insn (insn_t, int, int = -1);
 static void finish_insns (void);
 
 /* Various list functions.  */
@@ -4007,9 +4007,10 @@ get_seqno_by_succs (rtx insn)
   return seqno;
 }
 
-/* Compute seqno for INSN by its preds or succs.  */
+/* Compute seqno for INSN by its preds or succs.  Use OLD_SEQNO to compute
+   seqno in corner cases.  */
 static int
-get_seqno_for_a_jump (insn_t insn)
+get_seqno_for_a_jump (insn_t insn, int old_seqno)
 {
   int seqno;
 
@@ -4065,8 +4066,16 @@ get_seqno_for_a_jump (insn_t insn)
   if (seqno < 0)
     seqno = get_seqno_by_succs (insn);
 
-  gcc_assert (seqno >= 0);
+  if (seqno < 0)
+    {
+      /* The only case where this could be here legally is that the only
+	 unscheduled insn was a conditional jump that got removed and turned
+	 into this unconditional one.  Initialize from the old seqno
+	 of that jump passed down to here.  */
+      seqno = old_seqno;
+    }
 
+  gcc_assert (seqno >= 0);
   return seqno;
 }
 
@@ -4246,22 +4255,24 @@ init_insn_data (insn_t insn)
 }
 
 /* This is used to initialize spurious jumps generated by
-   sel_redirect_edge ().  */
+   sel_redirect_edge ().  OLD_SEQNO is used for initializing seqnos
+   in corner cases within get_seqno_for_a_jump.  */
 static void
-init_simplejump_data (insn_t insn)
+init_simplejump_data (insn_t insn, int old_seqno)
 {
   init_expr (INSN_EXPR (insn), vinsn_create (insn, false), 0,
 	     REG_BR_PROB_BASE, 0, 0, 0, 0, 0, 0,
 	     vNULL, true, false, false,
 	     false, true);
-  INSN_SEQNO (insn) = get_seqno_for_a_jump (insn);
+  INSN_SEQNO (insn) = get_seqno_for_a_jump (insn, old_seqno);
   init_first_time_insn_data (insn);
 }
 
 /* Perform deferred initialization of insns.  This is used to process
-   a new jump that may be created by redirect_edge.  */
-void
-sel_init_new_insn (insn_t insn, int flags)
+   a new jump that may be created by redirect_edge.  OLD_SEQNO is used
+   for initializing simplejumps in init_simplejump_data.  */
+static void
+sel_init_new_insn (insn_t insn, int flags, int old_seqno)
 {
   /* We create data structures for bb when the first insn is emitted in it.  */
   if (INSN_P (insn)
@@ -4288,7 +4299,7 @@ sel_init_new_insn (insn_t insn, int flags)
   if (flags & INSN_INIT_TODO_SIMPLEJUMP)
     {
       extend_insn_data ();
-      init_simplejump_data (insn);
+      init_simplejump_data (insn, old_seqno);
     }
 
   gcc_assert (CONTAINING_RGN (BLOCK_NUM (insn))
@@ -5575,14 +5586,14 @@ sel_merge_blocks (basic_block a, basic_block b)
 }
 
 /* A wrapper for redirect_edge_and_branch_force, which also initializes
-   data structures for possibly created bb and insns.  Returns the newly
-   added bb or NULL, when a bb was not needed.  */
+   data structures for possibly created bb and insns.  */
 void
 sel_redirect_edge_and_branch_force (edge e, basic_block to)
 {
   basic_block jump_bb, src, orig_dest = e->dest;
   int prev_max_uid;
   rtx jump;
+  int old_seqno = -1;
 
   /* This function is now used only for bookkeeping code creation, where
      we'll never get the single pred of orig_dest block and thus will not
@@ -5591,8 +5602,13 @@ sel_redirect_edge_and_branch_force (edge e, basic_block to)
               && !single_pred_p (orig_dest));
   src = e->src;
   prev_max_uid = get_max_uid ();
-  jump_bb = redirect_edge_and_branch_force (e, to);
+  /* Compute and pass old_seqno down to sel_init_new_insn only for the case
+     when the conditional jump being redirected may become unconditional.  */
+  if (any_condjump_p (BB_END (src))
+      && INSN_SEQNO (BB_END (src)) >= 0)
+    old_seqno = INSN_SEQNO (BB_END (src));
 
+  jump_bb = redirect_edge_and_branch_force (e, to);
   if (jump_bb != NULL)
     sel_add_bb (jump_bb);
 
@@ -5604,7 +5620,8 @@ sel_redirect_edge_and_branch_force (edge e, basic_block to)
 
   jump = find_new_jump (src, jump_bb, prev_max_uid);
   if (jump)
-    sel_init_new_insn (jump, INSN_INIT_TODO_LUID | INSN_INIT_TODO_SIMPLEJUMP);
+    sel_init_new_insn (jump, INSN_INIT_TODO_LUID | INSN_INIT_TODO_SIMPLEJUMP,
+		       old_seqno);
   set_immediate_dominator (CDI_DOMINATORS, to,
 			   recompute_dominator (CDI_DOMINATORS, to));
   set_immediate_dominator (CDI_DOMINATORS, orig_dest,
@@ -5623,6 +5640,7 @@ sel_redirect_edge_and_branch (edge e, basic_block to)
   edge redirected;
   bool recompute_toporder_p = false;
   bool maybe_unreachable = single_pred_p (orig_dest);
+  int old_seqno = -1;
 
   latch_edge_p = (pipelining_p
                   && current_loop_nest
@@ -5631,6 +5649,12 @@ sel_redirect_edge_and_branch (edge e, basic_block to)
   src = e->src;
   prev_max_uid = get_max_uid ();
 
+  /* Compute and pass old_seqno down to sel_init_new_insn only for the case
+     when the conditional jump being redirected may become unconditional.  */
+  if (any_condjump_p (BB_END (src))
+      && INSN_SEQNO (BB_END (src)) >= 0)
+    old_seqno = INSN_SEQNO (BB_END (src));
+
   redirected = redirect_edge_and_branch (e, to);
 
   gcc_assert (redirected && !last_added_blocks.exists ());
@@ -5651,7 +5675,7 @@ sel_redirect_edge_and_branch (edge e, basic_block to)
 
   jump = find_new_jump (src, NULL, prev_max_uid);
   if (jump)
-    sel_init_new_insn (jump, INSN_INIT_TODO_LUID | INSN_INIT_TODO_SIMPLEJUMP);
+    sel_init_new_insn (jump, INSN_INIT_TODO_LUID | INSN_INIT_TODO_SIMPLEJUMP, old_seqno);
 
   /* Only update dominator info when we don't have unreachable blocks.
      Otherwise we'll update in maybe_tidy_empty_bb.  */
diff --git a/gcc/testsuite/gcc.dg/pr60866.c b/gcc/testsuite/gcc.dg/pr60866.c
new file mode 100644
index 0000000..020878d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr60866.c
@@ -0,0 +1,18 @@
+/* { dg-do compile { target powerpc*-*-* ia64-*-* x86_64-*-* } } */
+/* { dg-options "-O -fselective-scheduling -fno-if-conversion -fschedule-insns"  } */
+
+int n;
+
+void
+foo (int w, int **dnroot, int **dn)
+{
+  int *child;
+  int *xchild = xchild;
+  for (; w < n; w++)
+    if (!dnroot)
+      {
+	dnroot = dn;
+	for (child = *dn; child; child = xchild)
+	  ;
+      }
+}

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