Re: [patch] Treegion formation for interblock scheduling

["Vladimir N. Makarov" <vmakarov at redhat dot com>]

mcrosier@ncsu.edu wrote:

> Hi,
> This patch introduces a new region formation type, referred to as
> Treegions, for interblock scheduling.  For the SPEC2K benchmarks, 86.2% of
> all basic blocks are contained within a multi-block regions (as compared
> to 61% (SPECINT) and 64% (SPECFP) for the current formation code after 2
> iterations of extend_rgns()).  An additional function to extend treegions
> (similar to extend_rgns(); not provided with this patch) increases this
> number to 89.6% by merging certain types of treegions.
>
> In addition to this patch, several additional enhancement are to follow
> that include a tail duplication implementation to increase region size
> (and therefore the opportunity for speculation) based on a cost analysis
> heuristic.  We refer to this heuristic as the Instantaneous Code Size
> Efficient (ICSE)) which takes into consideration the increase in IPC
> relative to code size increase.
>
> Additional details are to be presented at the GCC Summit at the end of the
> month.
>
> The changes have been tested on the trunk on ia64-unknown-linux-gnu with
> no regressions.
>
> Thanks,
>  Chad Rosier
>
>  
First of all, thanks for the efforts to improve insn scheduling in gcc.

I found that the patch gives a small improvement for SPEC2000 on ia64 
(0.2% for Int and 0.3% for FP), smaller code 0.15% for SPECInt, and no 
compiler time degradation.  So I think treegion should be switched on by 
default after checking (including bootstrap) other major ports 
(unfortunately the 1st insn scheduling is switched off for x86_64, so 
another platform would be ppc).  I hope your other coming patches will 
improve SPEC2000 more.

The patch itself is ok to commit to the mainline.  But you should fix 
whitespacing mentioned by Mike Stump.
Additionally you should put two spaces after period in comments and not 
finish line by an operator as in

+	      if(/* Don't traverse backedges */
+		 (ei_edge (current_ei)->flags & EDGE_DFS_BACK) ||
+		 /* Skip Exit block */
+		 (ei_edge (current_ei)->dest == EXIT_BLOCK_PTR) ||
+		 /* Skip if already in treegion */
+		 (TEST_BIT (in_treegion, ei_edge (current_ei)->dest->index)))

You should write it like
               if (...
                   || ...
                   || ...

All this requirements are described in GNU coding standards.  You can find it on gcc site.

Another important thing. You should wait when mainline is switched to stage 1.  Then, according to the policy, you can add a new functionality to gcc. 

> 2006-06-14  Chad Rosier  <mcrosier@ncsu.edu>
>
>        * common.opt (flag_sched1_use_treegions) : Add
> -fsched1-use-treegions.
>        * doc/invoke.text: Document -fsched1-use-treegions.
>        * sched-rgn.c (find_trgns): Define find natural treegions for
> scheduling.
>  
You could write just "New function".