[PATCH] D158368: [AMDGPU][MISCHED] GCNBalancedSchedStrategy.

[Alexander via Phabricator via llvm-commits]

alex-t added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp:1474
+  bool Result = false;
+  unsigned ScheduleLength = Top.getCurrCycle() + Bot.getCurrCycle();
+#ifndef NDEBUG
----------------
jrbyrnes wrote:
> I think this is a lower bound on what we have previously called ScheduleLength? 
Exactly. I realized that the ratio of the total stall cycles to the total amount of instructions better reflects the metric than the ratio of total stalls to the modeled length (i.e., the total amount of working cycles + the total amount of stalls).


================
Comment at: llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp:1487
+#endif
+  if (PrevMetric) {
+    unsigned Profit =
----------------
jrbyrnes wrote:
> Does this mean we always take the result from MaxOccupancy stage? I wonder if there is a way to have an initial target metric for the MaxxOcc stage?
The PrevMetric is zero only when we're in OccInitialSchedule stage because this stage is the first one. We compare the current stage metric with the previous best known on each next stage. Please note that we only record stage metrics if we don't revert the stage schedule.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158368