[PATCH, middle-end] Switch initializations conversion

[Jakub Jelinek]

Hi!

Just a couple of random style comments...

On Tue, Aug 28, 2007 at 11:12:25AM +0200, Martin Jambor wrote:
> 1) I added a testcase.

I think more testcases (also dg-do run to verify it works correctly) would
be desirable.

> --- gcc/tree-cswtch.c	(revision 0)
> +++ gcc/tree-cswtch.c	(revision 0)
> @@ -0,0 +1,907 @@
> +/* Switch Conversion converts variable initializations based on switch
> +   statements to initializations from a static array.  Copyright (C) 2006, 2007
> +   Free Software Foundation, Inc.  Contributed by Martin Jambor
> +   <jamborm@matfyz.cz>

Copyright line should be on its own line, Contributed note as well.

> +/* We never create arrays larger than the following constant (given in number
> +   of elements).  */
> +#define MAX_ARRAY_RANGE 0x2000
> +
> +/* We never create arrays  if the number of branches is not  at least the range
> +   divided by the following constant.  */
> +#define MAX_RANGE_BRANCH_RATIO 8

Have you considered making these parameters (see params.def)?

> +  for (t = &branch_list; 
> +       (*t != NULL_TREE) 
> +	 && (folder_evaluate_lt (CASE_LOW (cs), CASE_LOW (TREE_PURPOSE (*t))));
> +       t = &TREE_CHAIN (*t))
> +    ; /* empty loop body  */

This is ugly, the condition is too long.  Much better IMHO to rewrite this
as a while loop.

> +	      if (folder_evaluate_eq (TYPE_MIN_VALUE (TREE_TYPE (index)), 
> +				      index))
> +		{
> +		  /* must not underflow unsigned types */
> +		  break;
> +		}

One statement if bodies don't need curly braces.

> +      if (bb != final) 
> +	{
> +	  delete_basic_block (bb);
> +	} 

Likewise.

> +	  if (dump_file) 
> +	    {
> +	      fprintf (dump_file, "Processing of case %i failed\n", i);
> +	    }

Likewise...

> +static bool
> +convswitch_gate (void)
> +{
> +  return optimize >= 2;
> +}

I think you should add an option which controls this optimization
instead and by default set it for optimize >= 2, instead of doing it
unconditionally.

> @@ -564,6 +565,8 @@ init_optimization_passes (void)
>        NEXT_PASS (pass_merge_phi);
>        NEXT_PASS (pass_vrp);
>        NEXT_PASS (pass_dce);
> +
> +
>        NEXT_PASS (pass_dominator);
>        /* The only const/copy propagation opportunities left after
>  	 DOM should be due to degenerate PHI nodes.  So rather than

Why the whitespace change?

	Jakub