[PATCH] D30147: AMDGPU/SI: Add new SISched policy to reduce register usage

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:1526
+      for (SIScheduleBlock* Block : ReadyBlocks) {
+        if (CurrentPathRegUsage.find(Block) == CurrentPathRegUsage.end())
+          continue;
----------------
.count()


================
Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:1709
+
+struct SIBlockInfo {
+  // All Blocks that have to be scheduled first + Block
----------------
I think most of these are candidates for SmallSet/DenseSet etc.


================
Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:1747
+
+std::set<SIScheduleBlock *>
+SIScheduleBlockScheduler::findPathRegUsage(int SearchDepthLimit,
----------------
This whole function is long and repetitive. Can you refactor this into something more readable?


================
Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:1802
+    for (SIScheduleBlock* Block : SchedulableBlocks) {
+      struct SIBlockInfo BlockInfo = {};// TODO: check memset
+
----------------
Don't need struct or {}


================
Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:1807
+      for (SIScheduleBlock *Parent : Block->getPreds()) {
+        if (BlockInfos.find(Parent) != BlockInfos.end()) {
+          // The Parent was not scheduled before findPathRegUsage.
----------------
Doing a lot of repeat map lookups. Use the returned iterator of find


================
Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:2129
+    }
+    if (BestDiffVGPR == DiffVGPR && BestDiffSGPR == DiffSGPR) {
+      DEBUG(
----------------
This whole part looks like it can be factored into its own debug dump function


Repository:
  rL LLVM

https://reviews.llvm.org/D30147