[PATCH] D23688: AMDGPU/SI: Implement a custom MachineSchedStrategy

[Matt Arsenault via llvm-commits]

arsenm added inline comments.

================
Comment at: lib/Target/AMDGPU/GCNSchedStrategy.cpp:21
@@ +20,3 @@
+#include "SIRegisterInfo.h"
+#include "llvm/Target/TargetInstrInfo.h"
+
----------------
Should include SIInstrInfo instead

================
Comment at: lib/Target/AMDGPU/GCNSchedStrategy.cpp:30-55
@@ +29,28 @@
+
+static unsigned MaxWavesPerSGPRs(unsigned SGPRs,
+                                 const SISubtarget &ST) {
+
+  if (ST.getGeneration() >= SISubtarget::VOLCANIC_ISLANDS) {
+    if (SGPRs <= 80)
+      return 10;
+    if (SGPRs <= 88)
+      return 9;
+    if (SGPRs <= 100)
+      return 8;
+    return 7;
+  }
+  if (SGPRs <= 48)
+    return 10;
+  if (SGPRs <= 56)
+    return 9;
+  if (SGPRs <= 64)
+    return 8;
+  if (SGPRs <= 72)
+    return 7;
+  if (SGPRs <= 80)
+    return 6;
+  return 5;
+}
+
+static unsigned MaxWavesPerVGPRs(unsigned VGPRs) {
+  if (VGPRs <= 24)
----------------
We already have similar functions in the subtarget, so these should go there

================
Comment at: lib/Target/AMDGPU/GCNSchedStrategy.cpp:114-117
@@ +113,6 @@
+    TempTracker.getDownwardPressure(SU->getInstr(), Pressure, MaxPressure);
+  else
+    // FIXME: I think for bottom up scheduling, the register pressure is cached
+    // and can be retrieved by DAG->getPressureDif(SU).
+    TempTracker.getUpwardPressure(SU->getInstr(), Pressure, MaxPressure);
+ 
----------------
Braces

================
Comment at: lib/Target/AMDGPU/GCNSchedStrategy.cpp:193
@@ +192,3 @@
+  ReadyQueue &Q = Zone.Available;
+  for (ReadyQueue::iterator I = Q.begin(), E = Q.end(); I != E; ++I) {
+
----------------
Can this use a range loop?

================
Comment at: lib/Target/AMDGPU/GCNSchedStrategy.cpp:267-270
@@ +266,6 @@
+  // Pick best from BotCand and TopCand.
+  DEBUG(dbgs() << "Top Cand: ");
+  DEBUG(traceCandidate(BotCand));
+  DEBUG(dbgs() << "Bot Cand: ");
+  DEBUG(traceCandidate(TopCand));
+  SchedCandidate Cand;
----------------
One DEBUG block

================
Comment at: lib/Target/AMDGPU/GCNSchedStrategy.cpp:301-302
@@ +300,4 @@
+  }
+  DEBUG(dbgs() << "Picking: ");
+  DEBUG(traceCandidate(Cand));
+
----------------
Can be one DEBUG


https://reviews.llvm.org/D23688