Re: [PATCH] [4.7?] Fix PR 52250

[Richard Guenther <richard dot guenther at gmail dot com>]

2012/3/5 Andrey Belevantsev <abel@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 ?<abel@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;
>
> ?}