Re: rtlopt branch merge parts 3 & 5 -- recommented

[Richard Henderson <rth at redhat dot com>]

Reviewing only cfgloopmanip.c here.

On Sat, Feb 01, 2003 at 08:41:50PM +0100, Zdenek Dvorak wrote:
> remove_bbs (dom, bbs, nbbs)
>      dominance_info dom;
>      basic_block *bbs;
>      int nbbs;
> {
>   int i;
>   edge ae;
> 
>   for (i = 0; i < nbbs; i++)
>     {
>       edge next_ae;
>       for (ae = bbs[i]->succ; ae; ae = next_ae)
> 	{
> 	  next_ae = ae->succ_next;
> 	  remove_edge (ae);
> 	}
>       remove_bb_from_loops (bbs[i]);
>       delete_from_dominance_info (dom, bbs[i]);
>       flow_delete_block (bbs[i]);

flow_delete_block would take care of the edges.  No need for
special attention here, afaict.

> /* Fix placement of basic block BB inside loop hierarchy stored in LOOPS --
>    all successors of BB must belong to superloops of the loop L into that BB
>    belongs, with exception of edges into header of some loop, where the
>    outer loop of this loop must be superloop of L.

Awkward phrasing.  I had to read it several times before I understood.
How about something like

     For a basic block BB, a member of loop L, enforce the constraint that:
	(1) All successors of BB must belong to superloops of L, or
	(2) the successor is a header of L.

>    Returns 1 if we had
>    to move BB into other loop to enforce this condition, 0 if the placement
>    of BB was already correct (provided that placements of its successors are
>    correct).  */
> static int
> fix_bb_placement (loops, bb)

Prefer true and false over 1 and 0; prefer to use bool as the
return type of such functions.

> /* Removes branch beginning at edge E, i.e. remove basic blocks dominated by E
>    and update loop structure stored in LOOPS and dominators.  Return true if
>    we were able to remove the branch, false otherwise (and nothing is affected
>    then).  */
> bool
> remove_path (loops, e)

We should be consistent with "branch" vs "path".  I think I like
"path" better; "branch" makes me think of a jump instruction.
This affects the naming and comments of several functions in this file.

>   n = dfs_enumerate_from (loop->latch, 1, alp_enum_p, bbs, n_basic_blocks, loop->header);

Careful with overlong lines.

Please update the patch and re-post.


r~