[llvm-commits] [PATCH][SOPT] Fix assertion in Reassociate

[Shuxin Yang]

On 11/7/12 1:26 AM, Chandler Carruth wrote:
> On Tue, Nov 6, 2012 at 5:14 PM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:
>>> Two quick questions:
>>> 1. Is it generic enough to expose the class as a LLVM ADT?
>> Actually I used template to implement this worklist at beginning, hoping it
>> will be promoted into ADT.
> I think there are some real problems with the new worklist
> implementation that would also prevent it from going directly into the
> ADT, but honestly, we should probably fix those and get a proper
> FIFO-optimized worklist into the ADT if that's an important construct.
> Specifically, use of std::set seems really unfortunate here, and use
> of std::deque seems really bad. deque has pretty terrible performance
> overheads on a lot of STL implementations in the wild.
std::deque is the best container I could use. May I know the why it has 
terriable performance.
I thought I use std::map for set. Turn out I change to std::set for 
cleaner interface.
I'm sure std::map is using RB tree, it should be ok in terms of 
performance.
I don't know  how std::set is implemented and its performance.
Again, may I know the story of std::set.

>
>
> But I'd really like to understand exactly why SetVector didn't work.
> There are several places in LLVM that use it as a FIFO construct:
>
> // Note that the 'size' method is called on each trip as the worklist may grow.
> for (unsigned Idx = 0; Idx != Worklist.size(); ++Idx) {
>    Instruction *I = Worklist[Idx];
>    // ...
> }
As far as I can understand, SetVector is efficient in pop from front, 
and erase in the middle.