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

[Jeffrey Byrnes via Phabricator via llvm-commits]

jrbyrnes added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp:1474
+  bool Result = false;
+  unsigned ScheduleLength = Top.getCurrCycle() + Bot.getCurrCycle();
+#ifndef NDEBUG
----------------
I think this is a lower bound on what we have previously called ScheduleLength? 


================
Comment at: llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp:1487
+#endif
+  if (PrevMetric) {
+    unsigned Profit =
----------------
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?


================
Comment at: llvm/lib/Target/AMDGPU/GCNSchedStrategy.h:498
+    SchedStages.push_back(GCNSchedStageID::OccInitialSchedule);
+    SchedStages.push_back(GCNSchedStageID::UnclusteredHighRPReschedule);
+    SchedStages.push_back(GCNSchedStageID::ClusteredLowOccupancyReschedule);
----------------
I'm not sure I understand the purpose of UnclusteredHighRPReschedule stage in the context of a balanced scheduler.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158368