[llvm] [MachinePipeliner] Fix constraints aren't considered in certain cases (PR #95356)

Ryotaro KASUGA via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 20 22:21:16 PDT 2024


================
@@ -2420,47 +2420,48 @@ bool SwingSchedulerDAG::schedulePipeline(SMSchedule &Schedule) {
       // upon the scheduled time for any predecessors/successors.
       int EarlyStart = INT_MIN;
       int LateStart = INT_MAX;
-      // These values are set when the size of the schedule window is limited
-      // due to chain dependences.
-      int SchedEnd = INT_MAX;
-      int SchedStart = INT_MIN;
-      Schedule.computeStart(SU, &EarlyStart, &LateStart, &SchedEnd, &SchedStart,
-                            II, this);
+      Schedule.computeStart(SU, &EarlyStart, &LateStart, II, this);
       LLVM_DEBUG({
         dbgs() << "\n";
         dbgs() << "Inst (" << SU->NodeNum << ") ";
         SU->getInstr()->dump();
         dbgs() << "\n";
       });
-      LLVM_DEBUG({
-        dbgs() << format("\tes: %8x ls: %8x me: %8x ms: %8x\n", EarlyStart,
-                         LateStart, SchedEnd, SchedStart);
-      });
+      LLVM_DEBUG(
+          dbgs() << format("\tes: %8x ls: %8x\n", EarlyStart, LateStart));
 
-      if (EarlyStart > LateStart || SchedEnd < EarlyStart ||
-          SchedStart > LateStart)
+      if (EarlyStart > LateStart)
         scheduleFound = false;
-      else if (EarlyStart != INT_MIN && LateStart == INT_MAX) {
-        SchedEnd = std::min(SchedEnd, EarlyStart + (int)II - 1);
-        scheduleFound = Schedule.insert(SU, EarlyStart, SchedEnd, II);
-      } else if (EarlyStart == INT_MIN && LateStart != INT_MAX) {
-        SchedStart = std::max(SchedStart, LateStart - (int)II + 1);
-        scheduleFound = Schedule.insert(SU, LateStart, SchedStart, II);
-      } else if (EarlyStart != INT_MIN && LateStart != INT_MAX) {
-        SchedEnd =
-            std::min(SchedEnd, std::min(LateStart, EarlyStart + (int)II - 1));
-        // When scheduling a Phi it is better to start at the late cycle and go
-        // backwards. The default order may insert the Phi too far away from
-        // its first dependence.
-        if (SU->getInstr()->isPHI())
-          scheduleFound = Schedule.insert(SU, SchedEnd, EarlyStart, II);
-        else
-          scheduleFound = Schedule.insert(SU, EarlyStart, SchedEnd, II);
+      else if (EarlyStart != INT_MIN && LateStart == INT_MAX)
+        scheduleFound =
+            Schedule.insert(SU, EarlyStart, EarlyStart + (int)II - 1, II);
+      else if (EarlyStart == INT_MIN && LateStart != INT_MAX)
+        scheduleFound =
+            Schedule.insert(SU, LateStart, LateStart - (int)II + 1, II);
+      else if (EarlyStart != INT_MIN && LateStart != INT_MAX) {
+        // To keep original behaviour, start scheduling at the late cycle and go
+        // backwards when all scheduled predecessors are loop-carried
+        // output/order dependencies. Empirically, there are also cases where
+        // scheduling becomes possible with backward search.
+        if (Schedule.onlyHasLoopCarriedOutputOrOrderPreds(SU, this)) {
----------------
kasuga-fj wrote:

> does the bottom-up scheduling produces a better result for certain situations.
Yes. As far as I have checked with llvm-test-suite, there were cases where scheduling failed when stopping the backward search (about 1% of all cases). Removing lines 2446-2449 sometimes generated better schedule, but these were about a tenth of the cases where scheduling failed. Therefore I believe that keeping the original behavior is valuable.

In any case, as you mentioned, merging this process with isPHI() checks looks good to me. I'll fix it. 

https://github.com/llvm/llvm-project/pull/95356


More information about the llvm-commits mailing list