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: Minor cleanups in predict.c


> I did this about a week ago and haven't gotten a chance to check it in
> until now.  This fixes a number of whitespace problems and also does a general
> cleanup of code, making minor changes for clarity.  Not everything changes
> here is in the coding standards, but it makes the code a bit more
> consistent with the rest of GCC.
> 
> No functional changes are in this patch, but it was checked on
> alphaev56-dec-osf4.0c wiht other changes just in case.
Hi,

First of all, thanks for the cleanups.  I see I need to be more curefull
to make such cleanups unnecesary when modifying code.

> *************** dump_prediction (predictor, probability,
> *** 195,212 ****
>     fprintf (rtl_dump_file, "  %s heuristics%s: %.1f%%",
>   	   predictor_info[predictor].name,
> ! 	   used ? "" : " (ignored)",
> ! 	   probability * 100.0 / REG_BR_PROB_BASE);
>   
>     if (bb->count)
>       {
>         fprintf (rtl_dump_file, "  exec ");
> !       fprintf (rtl_dump_file, HOST_WIDEST_INT_PRINT_DEC,
> ! 	       (HOST_WIDEST_INT) bb->count);
>         fprintf (rtl_dump_file, " hit ");
> !       fprintf (rtl_dump_file, HOST_WIDEST_INT_PRINT_DEC,
> ! 	       (HOST_WIDEST_INT) e->count);
> !       fprintf (rtl_dump_file, " (%.1f%%)",
> ! 	       e->count * 100.0 / bb->count);
>       }
>     fprintf (rtl_dump_file, "\n");
>   }
> --- 200,214 ----
>     fprintf (rtl_dump_file, "  %s heuristics%s: %.1f%%",
>   	   predictor_info[predictor].name,
> ! 	   used ? "" : " (ignored)", probability * 100.0 / REG_BR_PROB_BASE);
>   
>     if (bb->count)
>       {
>         fprintf (rtl_dump_file, "  exec ");
> !       fprintf (rtl_dump_file, HOST_WIDEST_INT_PRINT_DEC, bb->count);
>         fprintf (rtl_dump_file, " hit ");
> !       fprintf (rtl_dump_file, HOST_WIDEST_INT_PRINT_DEC, e->count);
> !       fprintf (rtl_dump_file, " (%.1f%%)", e->count * 100.0 / bb->count);
>       }
> + 

I would preffer the casts to keep. The assumption that gcov_type ==
HOST_WIDEST_INT is bit fragile. For instance x86 do have some support for 128
bit aritmetic (inherited from 64bit arithmetic support of IA-32) and we plan to
make 128bit integer type available sometime later.  Then HOST_WIDEST_INT can
be made 128bit to make easier optimizing of SSE code and it will make sense to
add #if machinery to make gcov_type to get HOST_WIDE_INT instead.

So I am trying to consistently use the casts when converting gcov_type to
HOST_WIDEST_INT.
> *************** estimate_probability (loops_info)
> *** 414,423 ****
>   	  /* Look for block we are guarding (ie we dominate it,
>   	     but it doesn't postdominate us).  */
> ! 	  if (e->dest != EXIT_BLOCK_PTR
> ! 	      && e->dest != bb
>   	      && TEST_BIT (dominators[e->dest->index], e->src->index)
>   	      && !TEST_BIT (post_dominators[e->src->index], e->dest->index))
>   	    {
>   	      rtx insn;
>   	      /* The call heuristic claims that a guarded function call
>   		 is improbable.  This is because such calls are often used
> --- 411,420 ----
>   	  /* Look for block we are guarding (ie we dominate it,
>   	     but it doesn't postdominate us).  */
> ! 	  if (e->dest != EXIT_BLOCK_PTR && e->dest != bb

I tought there is prefference to write conditionals in a way that they eighter all
fit on single line or each has separate line unless they are somehow logically
related.

> *************** propagate_freq (head)
> *** 684,688 ****
>     for (; bb; bb = nextbb)
>       {
> !       volatile double cyclic_probability = 0, frequency = 0;
>   
>         nextbb = BLOCK_INFO (bb)->next;
> --- 680,684 ----
>     for (; bb; bb = nextbb)
>       {
> !       double cyclic_probability = 0, frequency = 0;

The volatile here is mandatory, as discussed in earlier comment (perhaps it
should be compiled here too), to avoid bootstrap misscomparisons on i386.  When
volatile is not present, the cyclic_probablility is computed in 80bit frequency
in the optimizer compiler making results slightly different that shows off with
tracer present on the cfg branch.

Honza


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