[PATCH] D83335: [ScheduleDAGRRList] Use std::*_heap() to keep candidate queue a heap.

[Florian Hahn via Phabricator via llvm-commits]

fhahn updated this revision to Diff 276450.
fhahn added a comment.

I updated the patch to limit the use of the heap to the src order comperator (which is used in combination with the MachineScheduler) and added extra verification to ensure the heap remains properly ordered and we pick the same candidate as with the existing heuristic.

With those verifications enabled, I managed to do a bootstrap build on X86 and built SPEC2000, SPEC2006 and MultiSource on X86 and AArch64 without a crash.

After taking a closer look at the source order operator, it looks like the guarantee a total ordering (they fall back to the NodeQueueId if all other criteria are equal and this ID is unique), so I don't think we would run into problems with multiple SUs having the same score.

There is a different potential issue though. For some comperators, the scoring of a candidate can change if a different node is scheduled. This does not seem like an issue on practice for the source order comperator, but it is for some of the other comperators, which is why this patch now limits the change to the source order comperator. After looking at the code for the source order comperator, it looks like the score could change after units are scheduled as well in some edge cases. This is not a big problem, as it would happen in a deterministic way and should only have very minor impact on the generated code, as the machinescheduler has the main responsibility for scheduling.

What do you think?

We might even go further and limit the source order comperator to just the IR ordering and the queue IDs, because the real scheduling should happen in the machine scheduler.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83335/new/

https://reviews.llvm.org/D83335

Files:
  llvm/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D83335.276450.patch
Type: text/x-patch
Size: 4158 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200708/7dc87855/attachment.bin>