[PATCH] D117952: [SLP] Restructure RescheduleHandling [NFC]

[Philip Reames via Phabricator via llvm-commits]

reames created this revision.
reames added reviewers: ABataev, nikic, fhahn.
Herald added subscribers: bollu, hiraditya, mcrosier.
reames requested review of this revision.
Herald added a project: LLVM.

This change clarifies, but does not actually change the handling for, how we handle a scheduling window extension.

Context:

- During normal pre-scheduling (i.e. what we do while walking the tree forming vector nodes), we only compute dependencies for instructions which are transitive users of a node in the vector tree.
- Our scheduling dependencies change based on the end of the current scheduling window.  When we change the boundary, we must ensure new dependencies are computed before the next schedule.
- Because of the first point, it's normal to have nodes reached in the transitive users of a new node which don't yet have valid dependencies.  As such, we can always leave nodes in the invalid dependency state so long as they haven't yet been scheduled.
- When dependencies are invalid, the node was not considered "ready" for scheduling.  As such, initialFillReadyList is a nop when no instruction has valid dependencies.

Let me explain the old code, because it was a tad non-obvious.

The case with a bundle is fairly straight forward.  We cleared dependencies and schedule.  Then initialFillReadyList is nop.  Then we schedule all the transitive users of the bundle.  And we've basically done a normal pre-schedule with a side effect of having to recompute dependencies and schedule for the entire DAG.

The case without a bundle is trickier, and the comments were misleading.  The key point is that initialFillReadyList does nothing when all nodes are the invalid dependency state.  As such, we hit the bottom, the readylist is empty, and we immediately exit having left both scheduling and dependencies cleared to their uncomputed states.

End result, we can do the same thing much more obviously by just resetting the info, and only attempting rescheduling if we have a bundle.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117952

Files:
  llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp


Index: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
===================================================================
--- llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -2362,6 +2362,8 @@
     bool isReady() const {
       assert(isSchedulingEntity() &&
              "can't consider non-scheduling entity for ready list");
+      assert(hasValidDependencies() &&
+             "can't schedule without dependencies");
       return UnscheduledDepsInBundle == 0 && !IsScheduled;
     }
 
@@ -7346,28 +7348,30 @@
     // recalculate all dependencies.
     // It is seldom that this needs to be done a second time after adding the
     // initial bundle to the region.
-    bool ReSchedule = false;
     if (ScheduleEnd != OldScheduleEnd) {
       for (auto *I = ScheduleStart; I != ScheduleEnd; I = I->getNextNode())
         doForAllOpcodes(I, [](ScheduleData *SD) { SD->clearDependencies(); });
-      ReSchedule = true;
-    }
-    if (ReSchedule) {
+
       resetSchedule();
-      initialFillReadyList(ReadyInsts);
-    }
-    if (Bundle) {
-      LLVM_DEBUG(dbgs() << "SLP: try schedule bundle " << *Bundle
-                        << " in block " << BB->getName() << "\n");
-      calculateDependencies(Bundle, /*InsertInReadyList=*/true, SLP);
+
+      // NOTE: At this point, we've cleared dependencies and schedule information
+      // we can not restart scheduling without computing dependencies again.  During
+      // prescheduling, we only build dependencies for things which are transitive
+      // users of a proposed node in the tree.  This means we leave large sections
+      // of the overall window without valid dependencies until final scheduling.
     }
+    if (!Bundle)
+      return;
+
+    LLVM_DEBUG(dbgs() << "SLP: try schedule bundle " << *Bundle
+               << " in block " << BB->getName() << "\n");
+    calculateDependencies(Bundle, /*InsertInReadyList=*/true, SLP);
 
-    // Now try to schedule the new bundle or (if no bundle) just calculate
-    // dependencies. As soon as the bundle is "ready" it means that there are no
-    // cyclic dependencies and we can schedule it. Note that's important that we
-    // don't "schedule" the bundle yet (see cancelScheduling).
-    while (((!Bundle && ReSchedule) || (Bundle && !Bundle->isReady())) &&
-           !ReadyInsts.empty()) {
+    // Now try to schedule the new bundle. As soon as the bundle is "ready" it
+    // means that there are no cyclic dependencies and we can schedule it. Note
+    // that it's important that we don't "schedule" the bundle yet (see
+    // cancelScheduling).
+    while (!Bundle->isReady() && !ReadyInsts.empty()) {
       ScheduleData *Picked = ReadyInsts.pop_back_val();
       if (Picked->isSchedulingEntity() && Picked->isReady())
         schedule(Picked, ReadyInsts);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D117952.402225.patch
Type: text/x-patch
Size: 2860 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220122/8486b981/attachment.bin>