This is an archive of the discontinued LLVM Phabricator instance.

Assert correct removal of SUnit in LatencyPriorityQueue
ClosedPublic

Authored by glasnak on Nov 15 2017, 8:18 AM.

Details

Summary

The LatencyPriorityQueue doesn't currently check whether the SU being removed really exists in the Queue.
This method fails quietly when SU is not found and removes the last element from the Queue, leading to unexpected behavior.

Unfortunately, this only occurs on our custom target, with the custom scheduler. In our case, when remove() is invoked, it removes the wrong SU at the end of the Queue, which is only discovered later when VerifyScheduledDAG() is invoked and finds that some nodes were not scheduled at all.

As this is only reproducible with a lot of proprietary code, I'm hopeful this assert is straightforward enough to not necessitate a test.

Disclaimer: I am a beginner and this is my first patch. All comments, complaints and criticisms are very welcome.

Diff Detail

Repository
rL LLVM

Event Timeline

glasnak created this revision.Nov 15 2017, 8:18 AM
bkramer accepted this revision.Nov 15 2017, 11:12 AM

lg. Do you have commit access?

This revision is now accepted and ready to land.Nov 15 2017, 11:12 AM

No, I don't think I have, would you commit it please?

This revision was automatically updated to reflect the committed changes.