Attached is a patch to fix a GLIBCXX_DEBUG error in ScheduleDAGRRList.
The problem is that calls to CapturePred may reprioritize elements in the
priority queue, violating streak weak ordering requirements.
To fix this, I introduced a reference wrapper for containers to obtain access
to the SUnitVec used by std::priority_queue. When CapturePred runs, it
calls updateNode which does a std::make_heap on the underlying SUnitVec.
This restores the correct ordering.
I believe this should also make the scheduler run more like it is supposed to.
Previously, the priority queue ordering was incorrect so we weren't
necessarly
scheduling correctly.
The container_reference_wrapper part may be controversial. So I want to
get some opinions on this before I submit it.
Thanks.
-Dave
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ScheduleDAG.patch
Type: text/x-diff
Size: 7731 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20071217/87b1bf6b/attachment.patch>
Hi Dave, This looks great to me! My only concern is the potential compile time impact. Do you see any? Also, please update the license portion to match what Chris sent out a couple of days ago. I don't see any issue with bringing Boost code into llvm tree. However, does it make sense to move the license to the top of the file? Chris? Evan On Dec 17, 2007, at 10:17 AM, David Greene wrote:> Attached is a patch to fix a GLIBCXX_DEBUG error in ScheduleDAGRRList. > > The problem is that calls to CapturePred may reprioritize elements > in the > priority queue, violating streak weak ordering requirements. > > To fix this, I introduced a reference wrapper for containers to > obtain access > to the SUnitVec used by std::priority_queue. When CapturePred > runs, it > calls updateNode which does a std::make_heap on the underlying > SUnitVec. > This restores the correct ordering. > > I believe this should also make the scheduler run more like it is > supposed to. > Previously, the priority queue ordering was incorrect so we weren't > necessarly > scheduling correctly. > > The container_reference_wrapper part may be controversial. So I > want to > get some opinions on this before I submit it. > > Thanks. > > -Dave > <ScheduleDAG.patch>_______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
On Monday 17 December 2007 13:39, Evan Cheng wrote:> My only concern is the potential compile time impact. Do you see any?I don't notice any, but then I'm not particularly looking for that either. I'll run some tests. I also accidentally included some debugging code I added to track down this prioritization problem (the queue dumping code). I'll remove that before I commit the change. I'll wait for Chris' comments re: licensing. -Dave