This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH]: Fix alias grouping of name tags.


On 12/10/06, Seongbae Park <seongbae.park@gmail.com> wrote:
On 12/9/06, Daniel Berlin <dberlin@dberlin.org> wrote:
> On 12/9/06, Eric Botcazou <ebotcazou@libertysurf.fr> wrote:
> > > I'm not sure why you are asking for this comment.  It sounds an awful
> > > lot like you want me to write "This code actually works and does what
> > > it is supposed to, regardless of what host you run it on", which seems
> > > wrong.  We generally document what we think isn't portable, not what
> > > is portable.
> >
> > We document what is non obvious, and I think that the ultimate non-dependency
> > on the host of this piece of code is non obvious.
>
> I've already said why i disagree that it is non-obvious, and not to
> belabor the pointer, but:
> I think it's actually obvious to since it is just used for sorting a
> list of pointers and removing duplicates.
> And the code there already says
>
> /* Replacing may aliases in name tags during grouping can end up with the
>    same SMT multiple times in the may_alias list.  It's quicker to
>    just remove them post-hoc than it is to avoid them during
>    replacement.  Thus, this routine sorts the may-alias list and
>    removes duplicates.  */
>
> ...
>     qsort (VEC_address (tree, aliases), VEC_length (tree, aliases),
>                      sizeof (tree), tree_pointer_compare);
> ...
>
> If you think it needs more docs, please document it the way that you
> feel makes more clear to the reader.
> Again, i can't figure out what part of "sorting a list of trees
> guaranteed to only have one pointer per UID (Something documented
> elsewhere) based on pointer equality, and removing duplicates"  you
> think this is not clear, so i can't do this!
>
> So please just document it how you feel is best so we can end this thread.
> --Dan

I quickly browsed through the code
and could not find any code that depends
on the order of items in var_ann_d::may_aliases.

Any such code, would, in fact, be very very broken.


In fact, if the length of the may_aliases vector is even moderately large, I can't imagine the linear search in various places would be acceptable. (I guess your other changes keeping the may_alias set size smaller would help quite a bit here).

The last time we tried using sparse bitmaps or other data structures, it ended up being slower during operand scanning. Basically all users use the may_aliases list through the vops it produces, which only consists of iterating over the alias list (and sorting the resulting vop list)

My personal feeling is that i'd rather not copy each alias set from a
bitmap into a vector, then iterate over the vector.  So it may be time
to do some timings on this again.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]