[PATCH] D46870: [MachineScheduler] Don't enforce some hazard checks pre-RA.

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa created this revision.
jonpa added a reviewer: atrick.
Herald added subscribers: JDevlieghere, javed.absar, MatzeB.

Currently, SchedBoundary::checkHazard() checks if an instruction begins or ends a group and if its micro-ops fits in the current group and may decide to put SU into Pending instead of Available based on this.

These are exact checks, but since so many things can happen with the code between pre-RA scheduling and final output, it seems far-fetched to put a priority pre-RA on this. It seems better to let such an instruction into Available and pick it if it e.g. helps with register pressure, and trust that post-RA scheduling will fix the grouping (which it most likely has to do anyway).

This patch introduces a new member SchedBoundary::IsPostRA and uses this to only do these checks post regalloc.

I also removed a TODO comment which seems already done.

This will be committed along with SystemZ changes soon, hopefully.


https://reviews.llvm.org/D46870

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


Index: lib/CodeGen/MachineScheduler.cpp
===================================================================
--- lib/CodeGen/MachineScheduler.cpp
+++ lib/CodeGen/MachineScheduler.cpp
@@ -1881,6 +1881,7 @@
 init(ScheduleDAGMI *dag, const TargetSchedModel *smodel, SchedRemainder *rem) {
   reset();
   DAG = dag;
+  IsPostRA = (DAG->MF.getRegInfo().getNumVirtRegs() == 0);
   SchedModel = smodel;
   Rem = rem;
   if (SchedModel->hasInstrSchedModel()) {
@@ -1931,27 +1932,27 @@
 /// simple counters that the scheduler itself maintains. It explicitly checks
 /// for instruction dispatch limitations, including the number of micro-ops that
 /// can dispatch per cycle.
-///
-/// TODO: Also check whether the SU must start a new group.
 bool SchedBoundary::checkHazard(SUnit *SU) {
   if (HazardRec->isEnabled()
       && HazardRec->getHazardType(SU) != ScheduleHazardRecognizer::NoHazard) {
     return true;
   }
 
-  unsigned uops = SchedModel->getNumMicroOps(SU->getInstr());
-  if ((CurrMOps > 0) && (CurrMOps + uops > SchedModel->getIssueWidth())) {
-    LLVM_DEBUG(dbgs() << "  SU(" << SU->NodeNum << ") uops="
-                      << SchedModel->getNumMicroOps(SU->getInstr()) << '\n');
-    return true;
-  }
+  if (IsPostRA) {
+    unsigned uops = SchedModel->getNumMicroOps(SU->getInstr());
+    if ((CurrMOps > 0) && (CurrMOps + uops > SchedModel->getIssueWidth())) {
+      LLVM_DEBUG(dbgs() << "  SU(" << SU->NodeNum << ") uops="
+                 << SchedModel->getNumMicroOps(SU->getInstr()) << '\n');
+      return true;
+    }
 
-  if (CurrMOps > 0 &&
-      ((isTop() && SchedModel->mustBeginGroup(SU->getInstr())) ||
-       (!isTop() && SchedModel->mustEndGroup(SU->getInstr())))) {
-    LLVM_DEBUG(dbgs() << "  hazard: SU(" << SU->NodeNum << ") must "
-                      << (isTop() ? "begin" : "end") << " group\n");
-    return true;
+    if (CurrMOps > 0 &&
+        ((isTop() && SchedModel->mustBeginGroup(SU->getInstr())) ||
+         (!isTop() && SchedModel->mustEndGroup(SU->getInstr())))) {
+      LLVM_DEBUG(dbgs() << "  hazard: SU(" << SU->NodeNum << ") must "
+                 << (isTop() ? "begin" : "end") << " group\n");
+      return true;
+    }
   }
 
   if (SchedModel->hasInstrSchedModel() && SU->hasReservedResource) {
@@ -2147,9 +2148,9 @@
   // exceed the issue width.
   const MCSchedClassDesc *SC = DAG->getSchedClass(SU);
   unsigned IncMOps = SchedModel->getNumMicroOps(SU->getInstr());
-  assert(
-      (CurrMOps == 0 || (CurrMOps + IncMOps) <= SchedModel->getIssueWidth()) &&
-      "Cannot schedule this instruction's MicroOps in the current cycle.");
+  assert((!IsPostRA || (CurrMOps == 0 ||
+                        (CurrMOps + IncMOps) <= SchedModel->getIssueWidth())) &&
+         "Cannot schedule this instruction's MicroOps in the current cycle.");
 
   unsigned ReadyCycle = (isTop() ? SU->TopReadyCycle : SU->BotReadyCycle);
   LLVM_DEBUG(dbgs() << "  Ready @" << ReadyCycle << "c\n");
@@ -2257,11 +2258,12 @@
   // This must be done after NextCycle has been adjust for all other stalls.
   // Calling bumpCycle(X) will reduce CurrMOps by one issue group and set
   // currCycle to X.
-  if ((isTop() &&  SchedModel->mustEndGroup(SU->getInstr())) ||
-      (!isTop() && SchedModel->mustBeginGroup(SU->getInstr()))) {
-    LLVM_DEBUG(dbgs() << "  Bump cycle to " << (isTop() ? "end" : "begin")
-                      << " group\n");
-    bumpCycle(++NextCycle);
+  if (IsPostRA &&
+      ((isTop() &&  SchedModel->mustEndGroup(SU->getInstr())) ||
+       (!isTop() && SchedModel->mustBeginGroup(SU->getInstr())))) {
+      LLVM_DEBUG(dbgs() << "  Bump cycle to " << (isTop() ? "end" : "begin")
+                        << " group\n");
+      bumpCycle(++NextCycle);
   }
 
   while (CurrMOps >= SchedModel->getIssueWidth()) {
Index: include/llvm/CodeGen/MachineScheduler.h
===================================================================
--- include/llvm/CodeGen/MachineScheduler.h
+++ include/llvm/CodeGen/MachineScheduler.h
@@ -618,6 +618,7 @@
     LogMaxQID = 2
   };
 
+  bool IsPostRA;
   ScheduleDAGMI *DAG = nullptr;
   const TargetSchedModel *SchedModel = nullptr;
   SchedRemainder *Rem = nullptr;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D46870.146782.patch
Type: text/x-patch
Size: 4173 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180515/e194ddc9/attachment.bin>