[PATCH] D98976: [CodeGen] Use ProcResGroup information in SchedBoundary

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

You folks all know more about scheduling than I do, but if you are in accord that is great. I can certainly verify that this improves the accuracy of the M7 schedule for the samples I've seen.

There are a number of loops where the if and else are very similar now. Is it possible to combine them, possibly with the use of some lambda functions? It might be worth having a getWriteProcRes that returns an iterator_range.

Some other nitpicks inline.



================
Comment at: llvm/lib/CodeGen/MachineScheduler.cpp:2058
+        HasUnbufferedResourceGroup |=
+            !(SchedModel->getProcResource(i)->BufferSize);
+        for (unsigned u = 0, ue = SchedModel->getProcResource(i)->NumUnits;
----------------
Don't need the brackets


================
Comment at: llvm/lib/CodeGen/MachineScheduler.cpp:2059
+            !(SchedModel->getProcResource(i)->BufferSize);
+        for (unsigned u = 0, ue = SchedModel->getProcResource(i)->NumUnits;
+             u != ue; ++u) {
----------------
u -> U, ue -> UE.


================
Comment at: llvm/lib/CodeGen/MachineScheduler.cpp:2256
     const MCSchedClassDesc *SC = DAG->getSchedClass(SU);
-    for (const MCWriteProcResEntry &PE :
-          make_range(SchedModel->getWriteProcResBegin(SC),
-                     SchedModel->getWriteProcResEnd(SC))) {
-      unsigned ResIdx = PE.ProcResourceIdx;
-      unsigned Cycles = PE.Cycles;
-      unsigned NRCycle, InstanceIdx;
-      std::tie(NRCycle, InstanceIdx) = getNextResourceCycle(ResIdx, Cycles);
-      if (NRCycle > CurrCycle) {
+    SmallVector<AdjustedResEntry, 16> ResTmp;
+    adjustProcResources(SC, ResTmp);
----------------
ResTmp -> umm, AdjustedResources?


================
Comment at: llvm/lib/CodeGen/MachineScheduler.cpp:2258
+    adjustProcResources(SC, ResTmp);
+    if (!ResTmp.size())
+      for (const MCWriteProcResEntry &PE :
----------------
I would add brackets to these, if they have large statements under them.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98976/new/

https://reviews.llvm.org/D98976