[PATCH] D130797: [AMDGPU] Implement pipeline solver for non-trivial pipelines

[Austin Kerbow via Phabricator via llvm-commits]

kerbowa added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp:641
+
+  // The number of create sched groups -- also used as SGID
+  int NumCreatedSchedGroups = 0;
----------------
typo: create


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp:644
 
-  // Used to track instructions that are already to added to a different
-  // SchedGroup with the same SyncID.
-  DenseMap<int, DenseSet<SUnit *>> SyncedInstrsMap;
+  // Used to track instructions that can be mapped to multiple sched groups
+  DenseMap<int, DenseMap<SUnit *, SmallVector<int, 4>>> SyncedInstrs;
----------------
I find it difficult to tell at a glance what the relationships these maps are defining. Maybe adding more description or breaking it up with a typedef would help.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp:747
+    // increase the cost
+    if (DAG->IsReachable(B, A))
+      continue;
----------------
Will missed edges ever be non-zero?  Try add edge will return false if B is a successor of A and that is already checked above. What is the intended meaning of a missed edge here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130797/new/

https://reviews.llvm.org/D130797