[patch] lno branch merge -- vectorizer patch #1

Zdenek Dvorak rakdver@atrey.karlin.mff.cuni.cz
Thu Aug 5 13:17:00 GMT 2004


Hello,

> >>>On Wed, Aug 04, 2004 at 10:16:17PM +0200, Sebastian Pop wrote:
> >>>>Seems like a merge bug.  We have kept the old name all along from
> >>>>tree-ssa days:
> >>>>
> >>>>2004-03-25  Diego Novillo  <dnovillo@redhat.com>
> >>>>
> >>>>	(pre_insert_on_edge): Rename from bsi_insert_on_edge_immediate.
> >>>
> >>>2004-07-09  Steven Bosscher  <stevenb@suse.de>
> >>>
> >>>       * tree-cfg.c (pre_insert_on_edge): Remove.
> >>>
> >>>
> >>>And in any case, I sincerely doubt you want the functionality that
> >>>this routine provided.
> >>>
> >>>
> >>
> >>Right, it was a bad hack for SSAPRE, which needed it because of some
> >>stupidity in the way the algorithm did insertions. You really want to
> >>just bsi_insert_on_edge and then bsi_commit_edge_insertions.
> >
> >no, there are some places where we certainly do not want this -- why
> >should we call a function that scans over *all* edges in the flowgraph
> >when we want a simple insertion on an edge???
> Most insertions onto an edge can be batched, and don't need to see the 
> actual results.

most insertions in loop optimizer cannot be batched, since all utility
functions keep all data structures and code representation accurate at
all times (and after experiences with coding the rtl loop optimizer I am
damn sure it is a good idea).

> Even for those that do need immediate results, have you ever seen 
> bsi_commit_edge_insertions on a profile taking any significant amount 
> of time?

No, but I have been using bsi_insert_on_edge_immediate to avoid this, so
it is not a surprise.  I am fairly sure I know at least one real-world
example when using this approach would cause very bad quadratic
behavior.

> Even if you did, you could just make it use a different datastructure, 
> that was O(number of insertions to do) rather than hanging the pending 
> statements off the CFG, making it O(E).

This seems like an unnecessarily great canon for this task.  As you
observed, in the case insertions are batched bsi_commit_edge_insertions
does not cause any performance problems, and for single insertions
we have a simple 10 lines long function that also works perfectly.
I do not see a reason for introducing a new more complicated mechanism.

> In any case, resurrecting a function that duplicates the entire 
> functionality seems like a really bad idea.

Why?  From the usage point of view,

new_bb = bsi_insert_on_edge_immediate (e, st);
if (new_bb)
  something;

Is much more pleasant and less error prone than

bsi_insert_on_edge (e, st);
bsi_commit_edge_inserts (&num);
if (num == 1)
  {
    new_bb = BASIC_BLOCK (last_basic_block - 1);
    something;
  }

If you insist, we may make bsi_insert_on_edge_immediate to be a wrapper
around bsi_insert_on_edge; bsi_commit_edge_inserts_1.  It makes it
a bit more complicated to find the newly created block and allocates
a new list item without any sane reason, which is why I haven't done it
so far.

Zdenek



More information about the Gcc-patches mailing list