[PATCH] D11885: AMDGPU/SI: Add SI Machine Scheduler

[Axel Davy via llvm-commits]

axeldavy added inline comments.

================
Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:650-652
@@ +649,5 @@
+    Blocks[BlockVariant] = CurrentBlocks;
+    return CurrentBlocks;
+  } else {
+    return B->second;
+  }
----------------
arsenm wrote:
> No return after else
Do you mean I should just remove the else ?

================
Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:870
@@ +869,3 @@
+  unsigned DAGSize = DAG->SUnits.size();
+  std::map<std::set<unsigned>, unsigned> ColorCombinations;
+
----------------
arsenm wrote:
> Having a set as the key to a map looks very strange. Is this really what you want?
Yes I want to assign colors to different sets. This looked like the best way to do.

================
Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:1556
@@ +1555,3 @@
+
+    if (SITII->isLowLatencyInstruction(SU->getInstr())) {
+      unsigned BestPos = LastLowLatencyUser + 1;
----------------
arsenm wrote:
> The normal naming convention is for this to just be TTI rather than SITTI
I use SITTI because TTI is already taken.

SITTI is TTI casted to SIInstrInfo*.
I use SITTI several times. It avoids casting again.



http://reviews.llvm.org/D11885