[PATCH] D63731: [AMDGPU] Prevent VGPR copies from moving across the EXEC mask definitions

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

I'm still not convinced just sticking EXEC reads on copies like this is a complete solution. Yes, we'll now insert the exec read on any copies that came out of isel, but any pass could introduce new copies. I guess if generic passes were more restricted when copies have implicit uses? It would require careful auditing of any generic code that can introduce copies, and may be more pessimizing than we really want



================
Comment at: lib/Target/AMDGPU/SIFoldOperands.cpp:502-508
+    MachineInstr::mop_iterator ImpOpI = UseMI->implicit_operands().begin();
+    MachineInstr::mop_iterator ImpOpE = UseMI->implicit_operands().end();
+    while (ImpOpI != ImpOpE) {
+      MachineInstr::mop_iterator Tmp = ImpOpI;
+      ImpOpI++;
+      UseMI->RemoveOperand(UseMI->getOperandNo(Tmp));
+    }
----------------
alex-t wrote:
> arsenm wrote:
> > Blindly stripping off any implicit_operands could be error prone. We do use other implicit operands on V_MOV_B32 for indirect register indexing
> This is done just before calling MachineInstr::addImplicitUseDefOperands().
> Also, this is under the "if" condition UseMI->isCopy() so not V_MOV exist so far.
This would be a lot simpler to do 
assert(UseMI->getNumOperands() == 3 && UseMI->getOperand(2) == exec), RemoveOperand(2).

I think this part should be split off into a separate patch to fix the extra implicit uses


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

https://reviews.llvm.org/D63731