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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:6166
+      return false;
+    const MachineBasicBlock *BA = InstA->getParent();
+    const MachineBasicBlock *BB = InstB->getParent();
----------------
skc7 wrote:
> arsenm wrote:
> > skc7 wrote:
> > > 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?
> > We used to do this in RPO. Not sure why this was changed.
> > 
> > Would it be easier to check all the operands of the one buffer instruction the first time you encounter it?
> The first time a buffer instruction is processed in the worklist for legalizeOperands, both rsrc and soffset operands wouldn't have vgprs assigned by the pass. Only one of them would have vgpr. V2S copy result used by the other operand still need to be processed in the worklist.
> 
> So, if legalizeOperands has to legalize soffset and rsrc together, V2S copies that occur prior to buffer instruction have to be processed in the worklist first.
Do they really need to be processed first? If you processed them at once, when those instructions are later processed they should see a copy with no uses which can be deleted


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

https://reviews.llvm.org/D143335