[PING] Patch to implement "Switch statement fdo reordering"

Mark Mitchell mark@codesourcery.com
Wed Aug 6 19:54:00 GMT 2008


Edmar Wienskoski wrote:

> I completed all the changes discussed bellow. Attached is a new
> version of the patch.

Sorry I dropped the ball here.

The structure is now much better.  I think that all that remains is some 
tidying up.

Here are some specific comments:

> +  int                   fdo_cost; /* Normalized profiling frequency */

This should say what is normalized relative to.  If I see the value "3" 
here, what does it mean?  What about -7?  (And if the latter doesn't 
make sense, then this should be an unsigned int.)

> +static inline rtx
> +promote_short_index (tree index_expr, int unsignedp)

All functions  need documentation for parameters and return value.   See 
other examples in GCC for appropriate conventions.

> +	       && fdo_switch_use_histogram_p(hist))

Formatting: need space before parenthesis.  Please check the rest of 
your patch for other possible occurrences of this problem.

> +	  /* Our histogram is limited by a fixed range,
> +	     exam the case list for cases out of this range */

Grammar: period after "range".  Diction: "Examine", not "Exam".

> +	      /* Get start of range */

Period and two spaces before close of comment.  Please review GNU coding 
standards and look through your whole patch to fix things that do not match.

> +	      if (TREE_INT_CST_HIGH(cn->low) < 0)
> +		{
> +		  count_negs += count_ranges;
> +		}

No braces.  See coding standards.

> +	  hcounters = (int *) xcalloc (1, ((int)hist->hdata.intvl.steps + 2)
> +				       * sizeof(int));

Should use XNEWVEC.  Freeing should be done with corresponding delete macro.

> +   Note that the table lookup scheme consists of specific code (5
> +   instructions in the PowerPC case) plus 1 or 2 compare/branches
> +   (depending on how the lower bound of the table would be handled)

Not clear.  Remove reference to PowerPC (and it's Power Architecture 
:-p) and explain more clearly what's going on.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713



More information about the Gcc-patches mailing list