Patchwork Fix PR45792, cris-elf build breakage from PR44374-fix "ifcvt/crossjump patch: Fix PR 42496, 21803"

login
register
mail settings
Submitter Bernd Schmidt
Date Sept. 27, 2010, 2:16 p.m.
Message ID <4CA0A74E.7010208@codesourcery.com>
Download mbox | patch
Permalink /patch/65871/
State New
Headers show

Comments

Bernd Schmidt - Sept. 27, 2010, 2:16 p.m.
On 09/27/2010 11:11 AM, Hans-Peter Nilsson wrote:
> Revision 164552 broke build for cris-elf, see PR45792.
> 
> Looking at the patch, I can't see how the "Try again"-codepath
> (see patch below) can ever work; the retry will try merging the
> insns starting with the NEXT_INSN after the old, merged, insns.
> But, the insns are merged, so the old NEXT_INSN is lost; the new
> NEXT_INSN is now move_before (the compare insn for the cbranch
> sequence), so you'll try moving the compare and jump to before
> the jump.  Which will cause reorder_insns to get stuck in a
> loop, after having set PREV_INSN(cmp) = NEXT_INSN (cmp) = cmp...
> 
> How did that pass testing?  Is there something I (we both) miss
> causing that code-path to never execute for non-cc0 targets?

It appears that it never triggers.  I'm pretty sure I tested that path
at some point, but I may have changed the currptr etc. bookkeeping
afterwards and broken it without noticing.

Here's a slightly different version, with the entire loop moved to an
earlier place (I saw crashes with your patch), and with an additional
free of the nextptr array.  This makes the code trigger a few times.
I've tested it on i686-linux; could you try on cris?  Ok if it passes.


Bernd

Patch

Index: cfgcleanup.c
===================================================================
--- cfgcleanup.c	(revision 164589)
+++ cfgcleanup.c	(working copy)
@@ -1944,7 +1944,7 @@  try_head_merge_bb (basic_block bb)
   basic_block final_dest_bb = NULL;
   int max_match = INT_MAX;
   edge e0;
-  rtx *headptr, *currptr;
+  rtx *headptr, *currptr, *nextptr;
   bool changed, moveall;
   unsigned ix;
   rtx e0_last_head, cond, move_before;
@@ -2077,6 +2077,7 @@  try_head_merge_bb (basic_block bb)
 
   currptr = XNEWVEC (rtx, nedges);
   headptr = XNEWVEC (rtx, nedges);
+  nextptr = XNEWVEC (rtx, nedges);
 
   for (ix = 0; ix < nedges; ix++)
     {
@@ -2132,6 +2133,14 @@  try_head_merge_bb (basic_block bb)
 
 	  /* Try again, using a different insertion point.  */
 	  move_before = jump;
+
+#ifdef HAVE_cc0
+	  /* Don't try moving before a cc0 user, as that may invalidate
+	     the cc0.  */
+	  if (reg_mentioned_p (cc0_rtx, jump))
+	    break;
+#endif
+
 	  continue;
 	}
 
@@ -2155,6 +2164,18 @@  try_head_merge_bb (basic_block bb)
 	    }
 	}
 
+      /* If we can't currently move all of the identical insns, remember
+	 each insn after the range that we'll merge.  */
+      if (!moveall)
+	for (ix = 0; ix < nedges; ix++)
+	  {
+	    rtx curr = currptr[ix];
+	    do
+	      curr = NEXT_INSN (curr);
+	    while (!NONDEBUG_INSN_P (curr));
+	    nextptr[ix] = curr;
+	  }
+
       reorder_insns (headptr[0], currptr[0], PREV_INSN (move_before));
       df_set_bb_dirty (EDGE_SUCC (bb, 0)->dest);
       if (final_dest_bb != NULL)
@@ -2170,16 +2191,18 @@  try_head_merge_bb (basic_block bb)
 	  if (jump == move_before)
 	    break;
 
-	  /* Try again, using a different insertion point.  */
+	  /* For the unmerged insns, try a different insertion point.  */
 	  move_before = jump;
+
+#ifdef HAVE_cc0
+	  /* Don't try moving before a cc0 user, as that may invalidate
+	     the cc0.  */
+	  if (reg_mentioned_p (cc0_rtx, jump))
+	    break;
+#endif
+
 	  for (ix = 0; ix < nedges; ix++)
-	    {
-	      rtx curr = currptr[ix];
-	      do
-		curr = NEXT_INSN (curr);
-	      while (!NONDEBUG_INSN_P (curr));
-	      currptr[ix] = headptr[ix] = curr;
-	    }
+	    currptr[ix] = headptr[ix] = nextptr[ix];
 	}
     }
   while (!moveall);
@@ -2187,6 +2210,7 @@  try_head_merge_bb (basic_block bb)
  out:
   free (currptr);
   free (headptr);
+  free (nextptr);
 
   crossjumps_occured |= changed;