[PATCH] D30153: AMDGPU/SI: Detect dependency types between blocks

[Valery Pykhtin via Phabricator via llvm-commits]

vpykhtin added inline comments.


================
Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:586
+    if (S.second == SIScheduleBlockLinkKind::Data)
+      dbgs() << "(Data Dep) ";
+    S.first->printDebug(false);
----------------
axeldavy wrote:
> arsenm wrote:
> > Debug printing not guarded by DEBUG. Looks like this problem existed already, so this whole block should be under DEBUG
> It is in a function printDebug, protected by #ifndef NDEBUG
Please provide full context diff next time (-U9999999), it helps to avoid such misunderstandings.

Better to use const reference to &S here.


================
Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:1365
       unsigned Height = 0;
-      for (SIScheduleBlock *Succ : Block->getSuccs()) {
-        if (Height < Succ->Height + 1)
-          Height = Succ->Height + 1;
+      for (std::pair<SIScheduleBlock*, SIScheduleBlockLinkKind> Succ :
+             Block->getSuccs()) {
----------------
const auto &Succ


================
Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:1367
+             Block->getSuccs()) {
+        if (Height < Succ.first->Height + 1)
+          Height = Succ.first->Height + 1;
----------------
In genereal please don't duplicate expressions. Compilers are good in CSE, but my eyes aren't :-)

This can be rewritten as Height = std::min(Height, Succ.first->Height + 1)


================
Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:1650
 void SIScheduleBlockScheduler::releaseBlockSuccs(SIScheduleBlock *Parent) {
-  for (SIScheduleBlock* Block : Parent->getSuccs()) {
-    --BlockNumPredsLeft[Block->getID()];
-    if (BlockNumPredsLeft[Block->getID()] == 0) {
-      ReadyBlocks.push_back(Block);
+  for (std::pair<SIScheduleBlock*, SIScheduleBlockLinkKind> Block :
+         Parent->getSuccs()) {
----------------
const auto &Block


================
Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:1653
+    --BlockNumPredsLeft[Block.first->getID()];
+    if (BlockNumPredsLeft[Block.first->getID()] == 0) {
+      ReadyBlocks.push_back(Block.first);
----------------
if (--BlockNumPredsLeft[Block.first->getID()] == 0)


Repository:
  rL LLVM

https://reviews.llvm.org/D30153