[PATCH] Do not set BB_SUPERBLOCK in loop-unroll.c:split_edge_and_insert
Sun Mar 4 14:57:00 GMT 2007
On 3/1/07, Zdenek Dvorak <email@example.com> wrote:
> > NB: I haven't bootstrapped&tested this as a whole yet, but I wanted to
> > show this as an alternative solution. Comments, please :-)
> I think this looks OK. With some more effort, it should not be
> that hard to keep dominance info/loop info in break_superblocks
> up-to-date, but I guess it is not needed, so I would not worry about
> that just now. I would suggest writing some comment to
> split_edge_and_insert, to document our (lack of) understanding of
> the situation for the future generations.
OK, so this turns out to be too difficult for me...
A desparate note
I keep running into the problem that I just have no way to figure out
where a fallthrough edge goes to in cfglayout mode. Why do we not want
JUMP_INSNs again in cfglayout mode? It drives me crazy.
Problems are the following:
* find_bb_boundaries removes all outgoing edges, so here we lose track
of where a fallthrough edge goes to in cfglayout mode. This is
unrecoverable information in the current implementation. My
work-around is to not remove the edges, but this assumes that those
edges are OK and I don't think I can rely on that, generally.
* In make_edges, cfgbuild.c:392-409, gcc wants to make an edge to
bb->next_bb. I want this for sub basic blocks that
find_sub_basic_blocks may have found, but I don't want this for the
original blocks -- again, I assume the existing outgoing edges are
valid. My work-around for now is to set BLOCK_ORIGINAL on the last
block that find_bb_boundaries finds (which may be the original block
if there are no sub basic blocks).
* Also in make_edges, gcc will try to make an edge from
ENTRY_BLOCK_PTR to ENTRY_BLOCK_PTR->next_bb, but there doesn't have to
be an edge there in cfglayout mode.
All this together results in the attached patch, which does not work,
unsurprisingly. I get functions "optimized" to empty functions,
indicating there are missing edges. I have no clue what else to do at
A happy note
I believe I've found some "proof" that the BB_SUPERBLOCK flag is
unnecessary in split_edge_and_insert:
1. There is a verify_flow_info_call in
loop-init.c:loop_optimizer_finalize(), which runs before
2. In verify_flow_info, we do not handle BB_SUPERBLOCK, so a control
flow insn inside a basic block would trigger an ICE even if the
BB_SUPERBLOCK flag is set on that block.
3. We have no reported ICEs of this kind.
Therefore, Zdenek, can you live with a patch that removes the line
that sets BB_SUPERBLOCK in split_edge_and_insert, and adds a comment
with your analysis of why it might be needed but we don't know for
A sad note
Even with the BB_SUPERBLOCK flag out of the way, I now run into new
problems, because of Ian's lower-subreg pass. Instead of fixing the
CFG in place when necessary, Ian makes the pass use
find_many_sub_basic_blocks to fix up some rare cases where additional
control flow edges may be required after lowering subregs.
And, you've guessed it, find_many_sub_basic_blocks doesn't work in
So now I can stay in cfglayout mode over even fewer passes, until I
(or, I'd really hope, Ian) will have taught lower-subreg to update the
CFG in place. Ian has already told me why he needed this, and I hope
we can find some time to solve the issue in a cleaner way.
Can we please put a "ban" on new uses of find_many_sub_basic_blocks?
This function should only be used by passes with no proper control
over the transformations that they do. As semi-proof of that, consider
the short list of passes, other than lower-subreg, that use it:
cfgexpand, the insn splitting passes, rtl-eh, and reload, i.e. the
usual list of passes that play "nice" on the CFG ;-)
The reason why find_many_sub_basic_blocks should be avoided is because
it basically destroys parts of the CFG in order to reconstruct it from
scratch. It destroys profile/predictions information in the process,
and that is unrecoverable. So this is IMHO a harmful Big Hammer for
lazy passes that should just work a little harder to do their job
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 4647 bytes
Desc: not available
More information about the Gcc-patches