[PATCH] D143335: [AMDGPU] Use instruction order in machine function to process workList of moveToVALU

[krishna chaitanya sankisa via Phabricator via llvm-commits]

skc7 added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:6166
+      return false;
+    const MachineBasicBlock *BA = InstA->getParent();
+    const MachineBasicBlock *BB = InstB->getParent();
----------------
foad wrote:
> Do you really need to guarantee the order between different basic blocks? If so, surely you need some kind of topological ordering, not just based on BB number. (And what if there are cycles in the CFG?)
I'm finding it difficult to compare two instructions from different basic blocks other than using BB number. Can we use dominator tree information? May be DFS numbers be used to compare them?


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:6185
+    // in the machine function.
+    llvm::stable_sort(Worklist, miCompare);
+    auto iter = Worklist.begin();
----------------
foad wrote:
> Sorting the whole worklist every time round this loop is not acceptable. You need to find a more efficient way of doing this.
Made changes in the current patch to use stl set with comparator for inserting and popping from worklist. This would not require to sort the worklist in every iteration. But the insertion and deletion from set have O(logn) time complexity.


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

https://reviews.llvm.org/D143335