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 <a...@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 <a...@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)
+ ;
+ }
+}
