This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] make find_taken_edge handle case with just default


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 <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]