[PATCH] D84328: [ScheduleDAGRRList] Limit number of candidates to explore.

[Florian Hahn via Phabricator via llvm-commits]

fhahn created this revision.
fhahn added reviewers: efriedma, paquette, niravd.
Herald added subscribers: hiraditya, kristof.beyls, MatzeB.
Herald added a project: LLVM.

Currently popFromQueueImpl iterates over all candidates to find the best
one. While the candidate queue is small, this is not a problem. But it
becomes a problem once the queue gets larger. For example, the snippet
below takes 330s to compile with llc -O0, but completes in 3s with this
patch.

define void @test(i4000000* %ptr) {
entry:

  store i4000000 0, i4000000* %ptr, align 4
  ret void

}

This patch limits the number of candidates to check to 1000. This limit
ensures that it never triggers for test-suite/SPEC2000/SPEC2006 on X86
and AArch64 with -O3, while still drastically limiting the compile-time
in case of very large queues.

It would be even better to use a binary heap to manage to queue
(D83335 <https://reviews.llvm.org/D83335>), but some heuristics change the score of a node in the queue
after another node has been scheduled. I plan to address this for
backends that use the MachineScheduler in the future, but that requires
a more careful evaluation. In the meantime, the limit should help users
impacted by this issue.

The patch includes a slightly smaller version of the motivating example
as test case, to guard against the issue.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84328

Files:
  llvm/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
  llvm/test/CodeGen/X86/stress-scheduledagrrlist.ll


Index: llvm/test/CodeGen/X86/stress-scheduledagrrlist.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/X86/stress-scheduledagrrlist.ll
@@ -0,0 +1,12 @@
+; RUN: llc -O0 -mtriple=x86_64-apple-macosx %s -o %t.s
+
+; Stress test for the list scheduler. The store will be expanded to a very
+; large number of stores during isel, stressing ScheduleDAGRRList. It should
+; compiles in a reasonable amount of time. Run with -O0, to disable most other
+; optimizations.
+
+define void @test(i1000000* %ptr) {
+entry:
+  store i1000000 0, i1000000* %ptr, align 4
+  ret void
+}
Index: llvm/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
+++ llvm/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
@@ -1838,13 +1838,15 @@
 
 template<class SF>
 static SUnit *popFromQueueImpl(std::vector<SUnit *> &Q, SF &Picker) {
-  std::vector<SUnit *>::iterator Best = Q.begin();
-  for (auto I = std::next(Q.begin()), E = Q.end(); I != E; ++I)
-    if (Picker(*Best, *I))
-      Best = I;
-  SUnit *V = *Best;
-  if (Best != std::prev(Q.end()))
-    std::swap(*Best, Q.back());
+  unsigned BestIdx = 0;
+  // Only compute the cost for the first 1000 items in the queue, to avoid
+  // excessive compile-times for very large queues.
+  for (unsigned I = 1, E = std::min(Q.size(), 1000ul); I != E; I++)
+    if (Picker(Q[BestIdx], Q[I]))
+      BestIdx = I;
+  SUnit *V = Q[BestIdx];
+  if (BestIdx + 1 != Q.size())
+    std::swap(Q[BestIdx], Q.back());
   Q.pop_back();
   return V;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D84328.279829.patch
Type: text/x-patch
Size: 1641 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200722/d673740a/attachment.bin>