[PATCH] D18602: AMDGPU/SI: Enable the post-ra scheduler

[Matt Arsenault via llvm-commits]

arsenm added inline comments.

================
Comment at: lib/Target/AMDGPU/GCNHazardRecognizer.cpp:107
@@ +106,3 @@
+void GCNHazardRecognizer::RecedeCycle() {
+  llvm_unreachable("hazard recoginizer does not support bottom-up scheduling.");
+}
----------------
Typo recogiznizer

================
Comment at: lib/Target/AMDGPU/GCNHazardRecognizer.cpp:138-139
@@ +137,4 @@
+
+  if (ST.getGeneration() != AMDGPUSubtarget::SOUTHERN_ISLANDS)
+    return 0;
+
----------------
A comment about why SI only would be helpful

================
Comment at: lib/Target/AMDGPU/GCNHazardRecognizer.h:18-21
@@ +17,6 @@
+#include "llvm/CodeGen/ScheduleHazardRecognizer.h"
+#include <cassert>
+#include <functional>
+#include <list>
+#include <map>
+
----------------
I think only <list> is needed here. The others aren't used or can be moved into the cpp file

================
Comment at: lib/Target/AMDGPU/GCNHazardRecognizer.h:30
@@ +29,3 @@
+
+class GCNHazardRecognizer : public ScheduleHazardRecognizer {
+
----------------
final

================
Comment at: lib/Target/AMDGPU/GCNHazardRecognizer.h:32
@@ +31,3 @@
+
+  // This variabled stores the instruction that has been emitted this cycle.
+  // It will be added to EmittedInstrs, when AdvanceCycle() or RecedeCycle() is
----------------
Typo: variabled

================
Comment at: lib/Target/AMDGPU/GCNHazardRecognizer.h:47
@@ +46,3 @@
+  GCNHazardRecognizer(const MachineFunction &MF);
+  // We can only issue on instruction be cycle.
+  bool atIssueLimit() const override { return true; }
----------------
Grammar: instruction be cycle

================
Comment at: lib/Target/AMDGPU/SIInstrInfo.cpp:828
@@ +827,3 @@
+  switch (MI.getOpcode()) {
+  default: return 1; // FIXME: Do wait states equal cycles?
+  case AMDGPU::S_NOP:
----------------
line break


http://reviews.llvm.org/D18602