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

Evan Cheng evan.cheng at apple.com
Mon Mar 3 13:26:46 PST 2008


Thanks.

Using std::set<> makes me queasy. :-) Too bad it doesn't seem possible  
to use SmallPtrSet (elements are not in sorted order).

Evan

On Mar 3, 2008, at 9:58 AM, Dan Gohman wrote:

> Hi Roman,
>
> Just one comment on this patch from me; in this change:
>
>>     SUnit *pop() {
>>       if (empty()) return NULL;
>> -      SUnit *V = Queue.top();
>> -      Queue.pop();
>> +      SUnit *V = *Queue.rbegin();
>> +      Queue.erase(V);
>>       return V;
>>     }
>
> The erase call here uses the "key" form of erase, so it incurs a
> lookup.  This can can be avoided by using the iterator form,
> something like this:
>
>   set::iterator i = prior(Queue.end());
>   SUnit *V = *i;
>   Queue.erase(i);
>
> Dan
>
> On Mar 3, 2008, at 3:06 AM, Roman Levenstein wrote:
>
>> Hi,
>>
>>
>> 2008/3/1, Evan Cheng <evan.cheng at apple.com>:
>>
>>> Let's try to solve one problem at a time.
>>>
>>> 1. Roman's depth / height patch calculation is obviously goodness.
>>> Please commit that first.
>>
>>
>> Here is the patch for ScheduleDAGList.cpp: computing priorities using
>> a linear algorithm. It uses the same idea as the height/depth
>> computation, i.e. dynamic programming. I took into account all
>> comments from Dan & Evan from last review.
>>
>> Additionally, I use now std::set instead of a priority queue. The
>> results are exactly the same as with the current SVN version - I
>> really compared it side-by-side during execution. Using std::set  
>> makes
>> operations such as removal from the middle of the queue much faster
>> and removes a bottleneck. Scheduling of very bigs BBs is now up to  
>> 3-4
>> times faster.
>>
>> Please, review this new version, if it is OK for submission.
>>
>> And there is one more patch to come, for the ScheduleDAGRRList.cpp,
>> which would also use std::set instead of priority queues, introduce
>> strict ordering and contain queue update changes as David suggested.
>>
>> -Roman
>> < 
>> ScheduleDAGList.patch>_______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits




More information about the llvm-commits mailing list