[llvm-commits] Fwd: Patch for review: Speeding up ScheduleDAG computations

Roman Levenstein romix.llvm at googlemail.com
Tue Mar 4 03:44:49 PST 2008


Hi,

2008/3/3, Dan Gohman <gohman at apple.com>:
>
>  On Mar 3, 2008, at 1:31 PM, Chris Lattner wrote:
>
>  > On Mar 3, 2008, at 1:26 PM, Evan Cheng wrote:
>  >> Thanks.
>  >>
>  >> Using std::set<> makes me queasy. :-) Too bad it doesn't seem
>  >> possible  to use SmallPtrSet (elements are not in sorted order).
>  >
>  > I'm sad to say that std::set is probably the best thing we have right
>  > now that provides priority queue + removal from the middle support.
>  >
>
> There's make_heap/push_heap/etc. in <algorithm> that let a
>  plain std::vector (or a SmallVector I guess) be used as a heap.

Yes, this is possible but produces much more overhead than std::set on
my tests. BTW, this approach is used in DAGISel.inc files generated by
tablegen. I tried to changed it to std::set as well and ,again, it
works much (25%-30%) faster  on BBs with few hundreds or thousends
instructions.

I'd like to understand better why Evan and Chris are some much against
std::set? From some mails I got the impression that std::set
implementation is very inefficient on Darwin or PowerPC? Is it
correct? Or do you dislike that fact that std::set uses dynamic memory
allocation extensively?

>  > Does anyone know of a better data structure to implement these
>  > operations?  Even if you don't volunteer to implement it, we can add
>  > it to the open projects page.

In principle, we need something like a balanced tree. And since we
want to be able to remove from the middle, I'd say that it should not
be mapped to something linear like std::vector, as it is done by
make_heap, since it involves quite some overhead and a lot of copying
around. Finding/creating a tree implementation is not such a big deal.
But again, why it would be better than std::set, that anyway seems to
be implemented internally as a read-black tree?

To conclude: Guys, I'm still waiting for a review and approval of the
two pending patches for ScheduleDAGList and ScheduleDAGRRList. Both of
them use std::set approach among other things.

- Roman



More information about the llvm-commits mailing list