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 <ja...@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.
--- 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;
+}
Jakub
