[PATCH] D39235: [MachineScheduler] minor refactoring

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa created this revision.
Herald added a subscriber: javed.absar.

I discovered a while back ago while reading the code in MachineScheduler.cpp that there was some duplicated code that was checking resource limits. I experimented with refactoring it, and this is my result. Does it look like an improvement I should commit?

NFC as far as I can see.


https://reviews.llvm.org/D39235

Files:
  include/llvm/CodeGen/MachineScheduler.h
  lib/CodeGen/MachineScheduler.cpp


Index: lib/CodeGen/MachineScheduler.cpp
===================================================================
--- lib/CodeGen/MachineScheduler.cpp
+++ lib/CodeGen/MachineScheduler.cpp
@@ -1831,6 +1831,11 @@
 
 SchedBoundary::~SchedBoundary() { delete HazardRec; }
 
+bool SchedBoundary::checkResourceLimit(unsigned Count, unsigned Lat) {
+  unsigned LFactor = SchedModel->getLatencyFactor();
+  return ((int)(Count - (Lat * LFactor)) > (int)LFactor);
+}
+
 void SchedBoundary::reset() {
   // A new HazardRec is created for each DAG and owned by SchedBoundary.
   // Destroying and reconstructing it is very expensive though. So keep
@@ -2085,10 +2090,7 @@
     }
   }
   CheckPending = true;
-  unsigned LFactor = SchedModel->getLatencyFactor();
-  IsResourceLimited =
-    (int)(getCriticalCount() - (getScheduledLatency() * LFactor))
-    > (int)LFactor;
+  updateIsResourceLimited();
 
   DEBUG(dbgs() << "Cycle: " << CurrCycle << ' ' << Available.getName() << '\n');
 }
@@ -2241,16 +2243,13 @@
           << " BotLatency SU(" << SU->NodeNum << ") " << BotLatency << "c\n");
   }
   // If we stall for any reason, bump the cycle.
-  if (NextCycle > CurrCycle) {
+  if (NextCycle > CurrCycle)
     bumpCycle(NextCycle);
-  } else {
+  else
     // After updating ZoneCritResIdx and ExpectedLatency, check if we're
     // resource limited. If a stall occurred, bumpCycle does this.
-    unsigned LFactor = SchedModel->getLatencyFactor();
-    IsResourceLimited =
-      (int)(getCriticalCount() - (getScheduledLatency() * LFactor))
-      > (int)LFactor;
-  }
+    updateIsResourceLimited();
+
   // Update CurrMOps after calling bumpCycle to handle stalls, since bumpCycle
   // resets CurrMOps. Loop to handle instructions with more MOps than issue in
   // one cycle.  Since we commonly reach the max MOps here, opportunistically
@@ -2435,10 +2434,9 @@
     OtherZone ? OtherZone->getOtherResourceCount(OtherCritIdx) : 0;
 
   bool OtherResLimited = false;
-  if (SchedModel->hasInstrSchedModel()) {
-    unsigned LFactor = SchedModel->getLatencyFactor();
-    OtherResLimited = (int)(OtherCount - (RemLatency * LFactor)) > (int)LFactor;
-  }
+  if (SchedModel->hasInstrSchedModel())
+    OtherResLimited = CurrZone.checkResourceLimit(OtherCount, RemLatency);
+
   // Schedule aggressively for latency in PostRA mode. We don't check for
   // acyclic latency during PostRA, and highly out-of-order processors will
   // skip PostRA scheduling.
Index: include/llvm/CodeGen/MachineScheduler.h
===================================================================
--- include/llvm/CodeGen/MachineScheduler.h
+++ include/llvm/CodeGen/MachineScheduler.h
@@ -682,6 +682,15 @@
 #endif
 
 public:
+  bool checkResourceLimit(unsigned Count, unsigned Lat);
+
+protected:
+  void updateIsResourceLimited() {
+    IsResourceLimited = checkResourceLimit(getCriticalCount(),
+                                           getScheduledLatency());
+  }
+
+public:
   /// Pending queues extend the ready queues with the same ID and the
   /// PendingFlag set.
   SchedBoundary(unsigned ID, const Twine &Name):


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D39235.120053.patch
Type: text/x-patch
Size: 3092 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171024/f458b23d/attachment.bin>