On Thu, 29 Jun 2017, Peter Bergner wrote:
> On 6/29/17 8:58 AM, Richard Biener wrote:
> > On Thu, 29 Jun 2017, Peter Bergner wrote:
> >
> >> On 6/29/17 4:03 AM, Richard Biener wrote:
> >>>
> >>> This refactors things a bit to make CFG cleanup handle switches with
> >>> just a default label. If we make sure to cleanup the CFG after
> >>> group_case_labels removes cases with just __builtin_unreachable ()
> >>> inside then this fixes the ICE seen in PR81994 as well.
> >>>
> >>> I wonder if find_taken_edge should generally handle successors
> >>> with __builtin_unreachable () -- OTOH that would get rid of those
> >>> too early I guess.
> >>
> >> Should we offer an early out of group_case_labels_stmt() for the
> >> fairly common case of new_size == old_size? There's no reason to
> >> execute the compress labels loop if we didn't combine any of the
> >> labels.
> >
> > We can also merge both loops, counting new_size upwards, storing
> > to label new_size if new_size != i ...
>
> Like this. I'm bootstrapping and regtesting the following patch on
> powerpc64le-linux. Ok if it survives?
Looks good to me.
Thanks,
Richard.
> Peter
>
> * tree-cfg.c (group_case_labels_stmt): Merge scanning and compressing
> loops. Remove now unneeded calls to gimple_switch_set_label() that
> just set removed labels to NULL_TREE.
>
> Index: gcc/tree-cfg.c
> ===================================================================
> --- gcc/tree-cfg.c (revision 249801)
> +++ gcc/tree-cfg.c (working copy)
> @@ -1679,13 +1679,13 @@ bool
> group_case_labels_stmt (gswitch *stmt)
> {
> int old_size = gimple_switch_num_labels (stmt);
> - int i, j, base_index, new_size = old_size;
> + int i, next_index, new_size;
> basic_block default_bb = NULL;
>
> default_bb = label_to_block (CASE_LABEL (gimple_switch_default_label
> (stmt)));
>
> /* Look for possible opportunities to merge cases. */
> - i = 1;
> + new_size = i = 1;
> while (i < old_size)
> {
> tree base_case, base_high;
> @@ -1699,23 +1699,21 @@ group_case_labels_stmt (gswitch *stmt)
> /* Discard cases that have the same destination as the default case.
> */
> if (base_bb == default_bb)
> {
> - gimple_switch_set_label (stmt, i, NULL_TREE);
> i++;
> - new_size--;
> continue;
> }
>
> base_high = CASE_HIGH (base_case)
> ? CASE_HIGH (base_case)
> : CASE_LOW (base_case);
> - base_index = i++;
> + next_index = i + 1;
>
> /* Try to merge case labels. Break out when we reach the end
> of the label vector or when we cannot merge the next case
> label with the current one. */
> - while (i < old_size)
> + while (next_index < old_size)
> {
> - tree merge_case = gimple_switch_label (stmt, i);
> + tree merge_case = gimple_switch_label (stmt, next_index);
> basic_block merge_bb = label_to_block (CASE_LABEL (merge_case));
> wide_int bhp1 = wi::add (base_high, 1);
>
> @@ -1727,9 +1725,7 @@ group_case_labels_stmt (gswitch *stmt)
> base_high = CASE_HIGH (merge_case) ?
> CASE_HIGH (merge_case) : CASE_LOW (merge_case);
> CASE_HIGH (base_case) = base_high;
> - gimple_switch_set_label (stmt, i, NULL_TREE);
> - new_size--;
> - i++;
> + next_index++;
> }
> else
> break;
> @@ -1742,23 +1738,22 @@ group_case_labels_stmt (gswitch *stmt)
> edge base_edge = find_edge (gimple_bb (stmt), base_bb);
> if (base_edge != NULL)
> remove_edge_and_dominated_blocks (base_edge);
> - gimple_switch_set_label (stmt, base_index, NULL_TREE);
> - new_size--;
> + i = next_index;
> + continue;
> }
> - }
>
> - /* Compress the case labels in the label vector, and adjust the
> - length of the vector. */
> - for (i = 0, j = 0; i < new_size; i++)
> - {
> - while (! gimple_switch_label (stmt, j))
> - j++;
> - gimple_switch_set_label (stmt, i,
> - gimple_switch_label (stmt, j++));
> + if (new_size < i)
> + gimple_switch_set_label (stmt, new_size,
> + gimple_switch_label (stmt, i));
> + i = next_index;
> + new_size++;
> }
>
> gcc_assert (new_size <= old_size);
> - gimple_switch_set_num_labels (stmt, new_size);
> +
> + if (new_size < old_size)
> + gimple_switch_set_num_labels (stmt, new_size);
> +
> return new_size < old_size;
> }
>
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
