diff mbox

Fix cfg_layout_merge_blocks (PR rtl-optimization/52139)

Message ID 20120207215747.GH18768@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Feb. 7, 2012, 9:57 p.m. UTC 
Hi!

On the following testcase we hit two bugs during cfglayout merge_blocks.

One is that b->il.rtl->header has some jumptable in it, followed by
barrier.  We call emit_insn_after_noloc to insert the whole b's header
after BB_END (a) and then delete_insn_chain it, with the intention that only
non-deletable insns like deleted label notes are kept around.  Unfortunately
delete_insn/remove_insn it uses isn't prepared to handle BARRIERs as part of
a bb (i.e. if BB_END is equal to some barrier because of the
emit_insn_after_noloc call, delete_insn_chain won't update BB_END properly).
As barriers aren't part of a BB, instead of adjusting remove_insn this patch
adjusts BB_END not to point to a barrier before calling delete_insn_chain.

The second bug is that remove_insn ICEs if deleting an insn with NEXT_INSN
NULL, unless that insn is part of a current sequence (or some sequence in
the sequence stack).  In the first version of the patch I've tried to
avoid calling delete_insn on insns that have NEXT_INSN NULL, but given that
having NULL NEXT_INSN is a pretty common situation when in cfglayout mode
if the insn is at BB_END, I think it is better to allow that in remove_insn.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2012-02-07  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/52139
	* emit-rtl.c (remove_insn): Allow removing insns
	with NEXT_INSN NULL, if they are at BB_END.
	* cfgrtl.c (cfg_layout_merge_blocks): If BB_END
	is a BARRIER after emit_insn_after_noloc, move BB_END
	to the last non-BARRIER insn before it.  Cleanup.

	* gcc.dg/pr52139.c: New test.


	Jakub
diff mbox

Patch

--- gcc/emit-rtl.c.jj	2012-02-07 16:05:47.913534092 +0100
+++ gcc/emit-rtl.c	2012-02-07 16:14:32.529734964 +0100
@@ -1,7 +1,7 @@ 
 /* Emit RTL for the GCC expander.
    Copyright (C) 1987, 1988, 1992, 1993, 1994, 1995, 1996, 1997, 1998,
    1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009,
-   2010, 2011
+   2010, 2011, 2012
    Free Software Foundation, Inc.
 
 This file is part of GCC.
@@ -3957,7 +3957,19 @@  remove_insn (rtx insn)
 	    break;
 	  }
 
-      gcc_assert (stack);
+      if (stack == NULL)
+	{
+	  /* In cfglayout mode allow remove_insn of
+	     insns at the end of bb.  */
+	  if (BARRIER_P (insn))
+	    {
+	      gcc_assert (prev);
+	      bb = BLOCK_FOR_INSN (prev);
+	    }
+	  else
+	    bb = BLOCK_FOR_INSN (insn);
+	  gcc_assert (bb && BB_END (bb) == insn);
+	}
     }
   if (!BARRIER_P (insn)
       && (bb = BLOCK_FOR_INSN (insn)))
--- gcc/cfgrtl.c.jj	2012-02-07 16:05:47.977533716 +0100
+++ gcc/cfgrtl.c	2012-02-07 17:03:52.925956927 +0100
@@ -2818,6 +2818,7 @@  static void
 cfg_layout_merge_blocks (basic_block a, basic_block b)
 {
   bool forwarder_p = (b->flags & BB_FORWARDER_BLOCK) != 0;
+  rtx insn;
 
   gcc_checking_assert (cfg_layout_can_merge_blocks_p (a, b));
 
@@ -2871,6 +2872,11 @@  cfg_layout_merge_blocks (basic_block a,
       rtx first = BB_END (a), last;
 
       last = emit_insn_after_noloc (b->il.rtl->header, BB_END (a), a);
+      /* The above might add a BARRIER as BB_END, but as barriers
+	 aren't valid parts of a bb, remove_insn doesn't update
+	 BB_END if it is a barrier.  So adjust BB_END here.  */
+      while (BB_END (a) != first && BARRIER_P (BB_END (a)))
+	BB_END (a) = PREV_INSN (BB_END (a));
       delete_insn_chain (NEXT_INSN (first), last, false);
       b->il.rtl->header = NULL;
     }
@@ -2878,40 +2884,28 @@  cfg_layout_merge_blocks (basic_block a,
   /* In the case basic blocks are not adjacent, move them around.  */
   if (NEXT_INSN (BB_END (a)) != BB_HEAD (b))
     {
-      rtx first = unlink_insn_chain (BB_HEAD (b), BB_END (b));
+      insn = unlink_insn_chain (BB_HEAD (b), BB_END (b));
 
-      emit_insn_after_noloc (first, BB_END (a), a);
-      /* Skip possible DELETED_LABEL insn.  */
-      if (!NOTE_INSN_BASIC_BLOCK_P (first))
-	first = NEXT_INSN (first);
-      gcc_assert (NOTE_INSN_BASIC_BLOCK_P (first));
-      BB_HEAD (b) = NULL;
-
-      /* emit_insn_after_noloc doesn't call df_insn_change_bb.
-         We need to explicitly call. */
-      update_bb_for_insn_chain (NEXT_INSN (first),
-				BB_END (b),
-				a);
-
-      delete_insn (first);
+      emit_insn_after_noloc (insn, BB_END (a), a);
     }
   /* Otherwise just re-associate the instructions.  */
   else
     {
-      rtx insn;
-
-      update_bb_for_insn_chain (BB_HEAD (b), BB_END (b), a);
-
       insn = BB_HEAD (b);
-      /* Skip possible DELETED_LABEL insn.  */
-      if (!NOTE_INSN_BASIC_BLOCK_P (insn))
-	insn = NEXT_INSN (insn);
-      gcc_assert (NOTE_INSN_BASIC_BLOCK_P (insn));
-      BB_HEAD (b) = NULL;
       BB_END (a) = BB_END (b);
-      delete_insn (insn);
     }
 
+  /* emit_insn_after_noloc doesn't call df_insn_change_bb.
+     We need to explicitly call. */
+  update_bb_for_insn_chain (insn, BB_END (b), a);
+
+  /* Skip possible DELETED_LABEL insn.  */
+  if (!NOTE_INSN_BASIC_BLOCK_P (insn))
+    insn = NEXT_INSN (insn);
+  gcc_assert (NOTE_INSN_BASIC_BLOCK_P (insn));
+  BB_HEAD (b) = NULL;
+  delete_insn (insn);
+
   df_bb_delete (b->index);
 
   /* Possible tablejumps and barriers should appear after the block.  */
--- gcc/testsuite/gcc.dg/pr52139.c.jj	2012-02-07 16:14:32.537734917 +0100
+++ gcc/testsuite/gcc.dg/pr52139.c	2012-02-07 16:14:32.537734917 +0100
@@ -0,0 +1,49 @@ 
+/* PR rtl-optimization/52139 */
+/* { dg-do compile } */
+/* { dg-options "-O -fno-tree-dominator-opts -fno-tree-fre" } */
+/* { dg-additional-options "-fpic" { target fpic } } */
+
+void *p;
+
+void
+foo (int a)
+{
+  switch (a)
+    {
+    case 0:
+    a0:
+    case 1:
+    a1:
+      p = &&a1;
+    case 2:
+    a2:
+      p = &&a2;
+    case 3:
+    a3:
+      p = &&a3;
+    case 4:
+    a4:
+      p = &&a4;
+    case 5:
+    a5:
+      p = &&a5;
+    case 6:
+    a6:
+      p = &&a6;
+    case 7:
+    a7:
+      p = &&a7;
+    case 8:
+    a8:
+      p = &&a8;
+    case 9:
+    a9:
+      p = &&a9;
+    case 10:
+    a10:
+      p = &&a10;
+    default:
+      p = &&a0;
+    }
+  goto *p;
+}