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

[Matt Arsenault via llvm-commits]

arsenm accepted this revision.
arsenm added a comment.
This revision is now accepted and ready to land.

LGTM with minor fixes


================
Comment at: lib/Target/AMDGPU/AMDGPUSubtarget.h:435
@@ +434,3 @@
+  unsigned getOccupancyWithNumSGPRs(unsigned SGPRs) const;
+  /// Return the maximum number of waves per SIMD for kernels using \p VGPRs VGPRs
+  unsigned getOccupancyWithNumVGPRs(unsigned VGPRs) const;
----------------
Line before comment

================
Comment at: lib/Target/AMDGPU/GCNSchedStrategy.cpp:15
@@ +14,3 @@
+#include "GCNSchedStrategy.h"
+#include "AMDGPU.h"
+#include "AMDGPUSubtarget.h"
----------------
I don't think this needs AMDGPU.h

================
Comment at: lib/Target/AMDGPU/GCNSchedStrategy.cpp:35-37
@@ +34,5 @@
+  const SIMachineFunctionInfo *MFI = MF.getInfo<SIMachineFunctionInfo>();
+  return std::min(std::min(ST.getOccupancyWithNumSGPRs(SGPRs),
+                           ST.getOccupancyWithNumVGPRs(VGPRs)),
+                           ST.getOccupancyWithLocalMemSize(MFI->getLDSSize()));
+}
----------------
I think this would be easier to read with the first min in a variable, std::min(MinRegOccupancy, LocalMemOccupancy)

================
Comment at: lib/Target/AMDGPU/GCNSchedStrategy.cpp:138
@@ +137,3 @@
+  const SIRegisterInfo *SRI = static_cast<const SIRegisterInfo*>(TRI);
+  const std::vector<unsigned> Pressure = RPTracker.getRegSetPressureAtPos();
+  unsigned SGPRPressure = Pressure[SRI->getSGPRPressureSet()];
----------------
Missing reference, maybe should just be an ArrayRef


https://reviews.llvm.org/D23688