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

[Evan Cheng]

On Mar 4, 2008, at 10:37 AM, Roman Levenstein wrote:

> Hi Evan,
>
> 2008/3/4, Evan Cheng <evan.cheng at apple.com>:
>>
>> On Mar 4, 2008, at 3:44 AM, Roman Levenstein wrote:
>>
>>> 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.
>>
>>
>> If you give me a patch, I'll test it on my end. Thanks.
>
> Fine. I'll provide this patch tomorrow. It is not a patch for the
> tablegen yet, it is just a small change of the code selector generated
> by the tablegen for the X86 target.

Ok, thanks.

>
>
>>> 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?
>>
>>
>> Right, it's very malloc intensive. That's the main issue.
>
> I think we may have a reasonable solution here. Remember that STL
> allows us to provide our own allocators as a template parameter for
> all collection classes including std::set. So, in principle we can use
> a cutom pool allocator or something like that to make std::set less
> memory intensive.
>
> BTW, here you can find a nice description about writing your own stl
> compatible allocator:
> http://www.codeguru.com/Cpp/Cpp/cpp_mfc/stl/article.php/c4079

Ok.

>
>
>>>
>>>
>>>>> 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?
>>
>>
>> Hard to say without testing. It might work better for small / medium
>> sized programs if it were designed to avoid malloc for smallish  
>> queues.
>
> See above. Custom allocator may solve our problems.
>
>>> 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.
>>
>>
>> I thought those were approved? We understanding std::set is probably
>> the best choice of ADT for now. Thanks!
>
> OK. I'll commit them tomorrow.
>
> But are you sure about the second one for the ScheduleDAGRRList?
> I mean this: http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080303/059078.html
>
> It also intorduces strict ordering for sorting functions and some
> other changes besides std::set. Would be nice to get a confirmation
> from others that it does not break any existing  test-cases and
> compiled programms still produce correct results.

I'll take a look.

Evan

>
>
> - Roman
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits