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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In D83335#2140138 <https://reviews.llvm.org/D83335#2140138>, @efriedma wrote:

> > Are you referring to using the heap only once the queue grows larger than a threshold or deciding what scheduling heuristics to enable based on the size?
>
> The scheduling heuristics.
>
> > The selection should be deterministic across different compilers/C++ STLs because the comparator enforces a total order.
>
> It's undefined behavior to call std::push_heap/std::pop_heap on an array that isn't a heap.  If the total order changes, that can break the heap property.  Not sure what the practical consequence would be on common STL implementations, but that seems scary enough that we want to ensure that can't happen.


Yeah we should avoid that. I'll take another look at the source order comparator, but I don't think we can rule out changing costs as of right now. Potentially changing the comparator for backends using the MachineScheduler is a bit bigger task. In the meantime I think I'll put up a patch that limits the number of candidates to scan linearly, to avoid a nasty quadratic compile-time case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83335