[PATCH] [4.7?] Fix PR 52250
Hello,
The PR is about selective scheduling not trying hard enough to find a
neighbor block to stick the note list from an empty block being removed.
Fixed by a) trying harder and b) making sure that if we fail to find the
right place, we just drop the notes instead of asserting.
Tested on x86-64 and ia64, approved by Alexander offline. No test as it
has quite insane number of flags that would likely stop reproducing the
core issue in a few weeks.
Richard, Jakub, I can commit this to trunk today or to wait until the
release. The patch is very safe and so can be committed in case we will
need an RC2, if you want one bug less in a release, but I would feel
equally happy to postpone it until 4.7.1 as the problematic situation is
quite rare (and probably requires pipelining outer loops to trigger which
is disabled with ia64/-O3).
Yours,
Andrey
2012-03-05 Andrey Belevantsev a...@ispras.ru
PR rtl-optimization/52250
* sel-sched-ir.c (maybe_tidy_empty_bb): Try harder to find a bb
to put note list into. Unconditionally call move_bb_info.
(move_bb_info): Do not assert the blocks being in the same region,
just drop the note list if they are not.
diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
index a93cd68..c53d2e1 100644
--- a/gcc/sel-sched-ir.c
+++ b/gcc/sel-sched-ir.c
@@ -3658,7 +3658,7 @@ sel_recompute_toporder (void)
static bool
maybe_tidy_empty_bb (basic_block bb)
{
- basic_block succ_bb, pred_bb;
+ basic_block succ_bb, pred_bb, note_bb;
VEC (basic_block, heap) *dom_bbs;
edge e;
edge_iterator ei;
@@ -3697,6 +3697,17 @@ maybe_tidy_empty_bb (basic_block bb)
pred_bb = NULL;
dom_bbs = NULL;
+ /* Save a pred/succ from the current region to attach the notes to. */
+ note_bb = NULL;
+ FOR_EACH_EDGE (e, ei, bb-preds)
+if (in_current_region_p (e-src))
+ {
+ note_bb = e-src;
+ break;
+ }
+ if (note_bb == NULL)
+note_bb = succ_bb;
+
/* Redirect all non-fallthru edges to the next bb. */
while (rescan_p)
{
@@ -3746,10 +3757,8 @@ maybe_tidy_empty_bb (basic_block bb)
else
{
/* This is a block without fallthru predecessor. Just delete it. */
- gcc_assert (pred_bb != NULL);
-
- if (in_current_region_p (pred_bb))
- move_bb_info (pred_bb, bb);
+ gcc_assert (note_bb);
+ move_bb_info (note_bb, bb);
remove_empty_bb (bb, true);
}
@@ -5231,10 +5240,9 @@ sel_remove_bb (basic_block bb, bool remove_from_cfg_p)
static void
move_bb_info (basic_block merge_bb, basic_block empty_bb)
{
- gcc_assert (in_current_region_p (merge_bb));
-
- concat_note_lists (BB_NOTE_LIST (empty_bb),
-BB_NOTE_LIST (merge_bb));
+ if (in_current_region_p (merge_bb))
+concat_note_lists (BB_NOTE_LIST (empty_bb),
+ BB_NOTE_LIST (merge_bb));
BB_NOTE_LIST (empty_bb) = NULL_RTX;
}
Re: [PATCH] [4.7?] Fix PR 52250
2012/3/5 Andrey Belevantsev a...@ispras.ru:
Hello,
The PR is about selective scheduling not trying hard enough to find a
neighbor block to stick the note list from an empty block being removed.
Fixed by a) trying harder and b) making sure that if we fail to find the
right place, we just drop the notes instead of asserting.
Tested on x86-64 and ia64, approved by Alexander offline. No test as it has
quite insane number of flags that would likely stop reproducing the core
issue in a few weeks.
Richard, Jakub, I can commit this to trunk today or to wait until the
release. The patch is very safe and so can be committed in case we will
need an RC2, if you want one bug less in a release, but I would feel equally
happy to postpone it until 4.7.1 as the problematic situation is quite rare
(and probably requires pipelining outer loops to trigger which is disabled
with ia64/-O3).
Note that 4.7 has branched, so you can commit to trunk. As it's a
non-standard option please defer backport until after the release of 4.7.0.
Thanks,
Richard.
Yours,
Andrey
2012-03-05 Andrey Belevantsev a...@ispras.ru
PR rtl-optimization/52250
* sel-sched-ir.c (maybe_tidy_empty_bb): Try harder to find a bb
to put note list into. Unconditionally call move_bb_info.
(move_bb_info): Do not assert the blocks being in the same region,
just drop the note list if they are not.
diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
index a93cd68..c53d2e1 100644
--- a/gcc/sel-sched-ir.c
+++ b/gcc/sel-sched-ir.c
@@ -3658,7 +3658,7 @@ sel_recompute_toporder (void)
static bool
maybe_tidy_empty_bb (basic_block bb)
{
- basic_block succ_bb, pred_bb;
+ basic_block succ_bb, pred_bb, note_bb;
VEC (basic_block, heap) *dom_bbs;
edge e;
edge_iterator ei;
@@ -3697,6 +3697,17 @@ maybe_tidy_empty_bb (basic_block bb)
pred_bb = NULL;
dom_bbs = NULL;
+ /* Save a pred/succ from the current region to attach the notes to. */
+ note_bb = NULL;
+ FOR_EACH_EDGE (e, ei, bb-preds)
+ if (in_current_region_p (e-src))
+ {
+ note_bb = e-src;
+ break;
+ }
+ if (note_bb == NULL)
+ note_bb = succ_bb;
+
/* Redirect all non-fallthru edges to the next bb. */
while (rescan_p)
{
@@ -3746,10 +3757,8 @@ maybe_tidy_empty_bb (basic_block bb)
else
{
/* This is a block without fallthru predecessor. Just delete it. */
- gcc_assert (pred_bb != NULL);
-
- if (in_current_region_p (pred_bb))
- move_bb_info (pred_bb, bb);
+ gcc_assert (note_bb);
+ move_bb_info (note_bb, bb);
remove_empty_bb (bb, true);
}
@@ -5231,10 +5240,9 @@ sel_remove_bb (basic_block bb, bool
remove_from_cfg_p)
static void
move_bb_info (basic_block merge_bb, basic_block empty_bb)
{
- gcc_assert (in_current_region_p (merge_bb));
-
- concat_note_lists (BB_NOTE_LIST (empty_bb),
- BB_NOTE_LIST (merge_bb));
+ if (in_current_region_p (merge_bb))
+ concat_note_lists (BB_NOTE_LIST (empty_bb),
+ BB_NOTE_LIST (merge_bb));
BB_NOTE_LIST (empty_bb) = NULL_RTX;
}
