[PATCH] D133572: [MachinePipeliner] Fix the interpretation of the scheduling model

[David Penry via Phabricator via llvm-commits]

dpenry added a comment.

Remarkably, I was just about to upload a patch to do this very thing for in-order non-DFA scheduling models, but this one has more functionality!  (It handles multi-cycle occupancy better, and the pipeliner-force-ii option will be quite useful for testing.)

Overall, I expect this to be helpful for Cortex-M7.  I can't speak to any side effects on models using DFA (Hexagon), or out-of-order non-DFA (PowerPC).  (NOTE: DFA is used by default; PowerPC and ARM targets turn it off.)

Do update the failing Thumb2 test outputs so I can comment on how/whether they need to be modified.  The use of pragmas in them to force the MII was precisely because of the problem this patch solves.

Also "unbuffered groups" -- resources which have subunits and whose BufferSize is zero -- are intended to consume resources differently -- or so the MachineScheduler code (SchedBoundary::getNextResourceCycle) implies.  At least in some cases, they do not consume resources for themselves, but choose between their subunits.  IIRC, this is used to model instructions having options in what resources they consume.  I don't think that getting this behavior in place is necessary for this patch, but could be in a follow-on patch.



================
Comment at: llvm/lib/CodeGen/MachinePipeliner.cpp:3079
+  if (NumScheduledInsts[positiveModulo(Cycle, InitiationInterval)] >=
+      IssueWidth) {
+    Result = false;
----------------
Will need to adjust for use of NumMicroOps as described in later comments


================
Comment at: llvm/lib/CodeGen/MachinePipeliner.cpp:3110
+
+  ++NumScheduledInsts[positiveModulo(Cycle, InitiationInterval)];
+  reserveResources(SCDesc, Cycle);
----------------
Should increment by SCDesc->NumMicroOps (see comment below while calculating ResMII)



================
Comment at: llvm/lib/CodeGen/MachinePipeliner.cpp:3226
+    const MCSchedClassDesc *SCDesc =
+        SM.getSchedClassDesc(MI->getDesc().getSchedClass());
+    if (!SCDesc->isValid())
----------------
Use SM.resolveSchedClass here so that variant scheduling classes can be resolved.


================
Comment at: llvm/lib/CodeGen/MachinePipeliner.cpp:3231
+    LLVM_DEBUG(if (SwpDebugResource) DAG->dumpNode(*DAG->getSUnit(MI)););
+    ++NumInsts;
+    for (const MCWriteProcResEntry &PRE :
----------------
Increment by SCDesc->NumMicroOps, as NumMicroOps represents the amount of issue width taken by the instruction (at least that's how MachineScheduler interprets it).




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133572