bb-reorder rewrite
Jason Eckhardt
jle@cygnus.com
Wed May 24 16:52:00 GMT 2000
On Mon, 22 May 2000, Richard Henderson wrote:
>
> What happened is that jump2 decided to fix up several of the jumps
> by *undoing* the rearrangement bb-reorder had effected.
>
> Given that this sort of thing is possible, and that long-term we'd
> like to reduce or eliminate jump_optimize, it seems clear that
> bb-reorder should do the entire transformation itself.
Right. I had also thought about this since I wanted to try reordering
blocks earlier. But cleanup_cfg will also undo things. Now that should
be solved with this work.
>
>
> "Profile Guided Code Positioning"
> Pettis and Hanson; PLDI '90.
>+
>+ TODO:
>+
>+
>+ (3) Invent a method by which sufficiently non-predicted code can
>+ be moved to either the end of the section or another section
>+ entirely. Some sort of NOTE_INSN note would work fine.
>+
This is also mentioned in the above paper. They actually collect
all the not-predicted code hunks (from every function) together into their
own section.
Then all the functions (with the frequently executed code left) are able
to be crammed together, fitting more of them to each memory page.
>! /* ??? Get rid of this loop, and track which blocks are not yet
>! placed more directly, so as to avoid the O(N^2) worst case.
>! Perhaps keep a doubly-linked list of all to-be-placed blocks;
>! remove from the list as we place. The head of that list is
>! what we're looking for here. */
>
>! for (i = 0; i <= nbb_m1; ++i)
> {
>! basic_block bb = BASIC_BLOCK (i);
>! if (! RBI (bb)->visited)
> {
>! next = bb;
>! break;
> }
> }
Could we just make this whole body "while (next)" instead of coding
the loop with "goto restart"?
>! restart:
>! RBI (prev)->next = bb;
>! new_index = RBI (prev)->index + 1;
>! RBI (bb)->index = new_index;
[...]
>
>! /* In the absense of prediction, disturb things as little as possible
of a prediction
>! by selecting the old "next" block from the list of successors. If
>! there had been a fallthru edge, that will be the one. */
[...]
>
>! /* Make sure we didn't select a silly next block. */
>! if (!next || next == EXIT_BLOCK_PTR || RBI (next)->visited)
! next
[...]
>! }
>! if (next)
>! {
>! bb = next;
>! goto restart;
>! }
>
>! return prev;
> }
>
This section looks suspect:
>! /* Disconnect the existing edge from the target block. */
>! {
>! edge *pe = &e_fall->dest->pred;
>! for (e = *pe; e != e_fall; pe = &e->pred_next, e = *pe)
>! continue;
>! *pe = e->pred_next;
>! }
>!
>! /* Link to new block. */
>! make_edge (NULL, nb, e_fall->dest, 0);
>! nb->pred = e_fall;
>! e_fall->dest = nb;
It appears that e_fall gets disconnected properly, but its pred_next
field is never updated. Perhaps it should be zeroed?
Perhaps this code should be factored out into something like
a "redirect_edge" or "delete_edge" function. I can see this being useful
elsewhere in the future.
jason
More information about the Gcc-patches
mailing list