[PATCH] Do not set BB_SUPERBLOCK in loop-unroll.c:split_edge_and_insert

Steven Bosscher stevenb.gcc@gmail.com
Sun Mar 4 14:57:00 GMT 2007


On 3/1/07, Zdenek Dvorak <rakdver@atrey.karlin.mff.cuni.cz> wrote:
> Hello,
>
> > 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.

Hi Zdenek,

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
this point.


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
break_superblocks.
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
sure?



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
cfglayout mode.

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
right.</complaint> ;-)

Gr.
Steven
-------------- next part --------------
A non-text attachment was scrubbed...
Name: BB_SUPERBLOCK_mess_t2.diff.gz
Type: application/x-gzip
Size: 4647 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20070304/b8d8363f/attachment.bin>


More information about the Gcc-patches mailing list