This is the mail archive of the
mailing list for the GCC project.
Re: rtlopt branch merge parts 3 & 5 -- recommented
- From: Richard Henderson <rth at redhat dot com>
- To: Zdenek Dvorak <rakdver at atrey dot karlin dot mff dot cuni dot cz>
- Cc: gcc-patches at gcc dot gnu dot org, jh at suse dot cz, kenner at vlsi1 dot ultra dot nyu dot edu
- Date: Thu, 6 Feb 2003 14:08:43 -0800
- Subject: Re: rtlopt branch merge parts 3 & 5 -- recommented
- References: <20030201194150.GA2708@atrey.karlin.mff.cuni.cz>
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). */
> 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.