Re: [patch] Rewrite building of virtual operands

[Zdenek Dvorak <rakdver at atrey dot karlin dot mff dot cuni dot cz>]

Hello,

> > At the moment, tree-ssa-opfinalize.h forces us to build virtual operands
> > in a way that is similar to that for virtual operands, i.e., to
> > accumulate them in a vector.  However, we know that for each DECL, there may be
> > only one virtual operand, and we want them to be sorted by decl_uid in
> > the list of virtual operands of the statement; achieving this is a bit
> > clumsy with the current representation -- bitmap is the appropriate type
> > for representing the object that satisfies these two properties.
> 
> Its done that way for a reason. I spent a *lot* of time tuning this
> code. This lies underneath almost everything ssa does, so minor changes
> can impact the speed of all ssa passes, especially in the more
> pathological cases. By using an sbitmap, you are going to have a lot of
> additional allocates and frees for the sbitmap's...

I use bitmaps, not sbitmaps.  Sbitmaps are slow to iterate over, so
their use definitely is not appropriate.

> the in_list bits of
> the annotations are already allocated and available. I agree they are a
> touch awkward, but they are faster.   Did you try to time this patch?
> its bound to slow down the operand cache.  I got about a 20% slowdown on
> cplusplus_grammer.cc , less on .c files and I'm sure its much worse on
> the nasty ones that I didn't get around to tracking down. (the ones in
> bugzilla where large percentages of compile time is operand processing)

are you speaking about the timings for this patch, or your patch using
sbitmaps? Where can I find this cplusplus_grammer.cc file?

I have tried measuring times for some c files (combine.i and loop.i),
and I did not notice any increase in times for TV_TREE_OPS timevar.

> Im not sure why you think everything needs to be changed in order to do
> what you want. I mentioned a fairly simply way of doing it that wouldn't
> cost much time, nor require reimplementing anything.

I am sure that it is not necessary.  However, it seems like a logical
cleanup to me -- it makes tree-ssa-operands.c shorter by 100 lines and
I find the new code easier to follow (of course, I have written it, so
that's hardly objective).  Unless there really are performance
reasons against it, I prefer cleaning things up rather than adding
new hacks.

Zdenek