This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [CFG] Loop represenation & new loop optimizer works
Hello.
> > This patch implements new loop represenation. It also makes some changes
> > to support moving to new loop optimizer.
>
> I guess it is better to place it in cfgloopanal.c I made for such a simple
> loop code to separate fucntion contstruct_preheaders.
>
> > + /* Used by dfs_enumerate_from; keep this one zero! */
> > + #define BB_VISITED 8
>
> We generally don't polute flags by pass dependent data. Can't you
> use the AUX field to store the flag as it is done in cfgcleanup/ifcvt?
This is not pass dependent; dfs_enumerate_from is basis for get_loop_body and
is used on few other functions that are used in situations aux field is already
reserved for some pass specific data (and in more than one pass, so I can't add
my flag to them).
> > ! {
> > ! BASIC_BLOCK (i)->aux = NULL;
> > ! BASIC_BLOCK (i)->global_live_at_start = NULL;
> > ! BASIC_BLOCK (i)->global_live_at_end = NULL;
> > ! }
>
> Why exactly this is needed? The basic block structure is zeroed before
> used, so they should be 0 by default.
Then something doesn't work, as it happened to me that these fields had
non-NULL values just after find_basic_blocks and it made problems in
cfgcleanup.
> > /* Scan basic block BB for possible BB boundaries inside the block
> > ! and create new basic blocks in the progress. Adds new bbs to NW,
> > ! their number to NNW, and updates size of NW array (MNW) if needed. */
> >
> > static void
> > ! find_bb_boundaries (bb, nw, nnw, mnw)
> > basic_block bb;
> > + basic_block **nw;
> > + int *nnw;
> > + int *mnw;
>
> Can't you use the BB_NEW flags I created recently? They should give you
> info about newly created basic block you need to update loop structure for.
> It needs search trought all basic blocks instead of only those changed to
> find the falgs having potential time complexity problem, but searching
> single flag in BB is cheap and I believe it is cleaner and easier
> to understand interface.
I thought it too slow to have to pass through all bbs after emiting few
on some edge; but this isn't critical, so I can rework it this way if
you want to.
> > + if (loops)
> > + {
> > + /* bb cannot be loop header, as it only has one entry
> > + edge. It could be a loop latch. */
> > + if (bb->loop_father->header == bb)
> > + abort ();
> > +
> > + if (bb->loop_father->latch == bb)
> > + bb->loop_father->latch = bb->pred->src;
> > +
> > + if (get_immediate_dominator
> > + (loops->cfg.dom, bb->succ->dest) == bb)
> > + set_immediate_dominator
> > + (loops->cfg.dom, bb->succ->dest, bb->pred->src);
> > +
> > + remove_bb_from_loops (bb);
> > + }
> > +
>
> Isn't exactlyt he same code needed at different places too?
> I guess it should be possible to update datastructure after general BB
> is removed and then I think we should have function doing that.
I don't need exactly this anywhere just now; and I don't think I will
(updating dominators after removing some bbs/edges is generally hard and
differs from place to place a lot. I could make some generic code for this,
but in simple cases like this it probably isn't useful).
> Also perhaps we can get the loop infromation from BB structure that contains
> pointer to loop it is contained in instead of passing it everwhere.
> That would make possible to teach delete_block do the trick w/o altering
> the current interface. The loop structure can be seen as property of CFG
> graph, as we see CFG as the property of insn chain.
I pass loops here because of loops->cfg.dom field (which isn't used now;
but it is probable that I will need to pass something like this once
Pavel is done with reworking structure for dominators).
> > ! if (e)
> > ! {
> > ! if (e->flags & EDGE_FALLTHRU)
> > ! redirect_edge_succ (e, new_bb);
> > ! else
> > ! redirect_edge_and_branch (e, new_bb);
> > ! }
> Perhaps you should put the other conditional updates of frequency to this
> condtiional to avoid ?: construct.
I don't understand?
Zdenek