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

[Alexander via Phabricator via llvm-commits]

alex-t marked an inline comment as done.
alex-t added inline comments.


================
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));
+    }
----------------
arsenm wrote:
> 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
As you mentioned, there may be another copy makers besides the legalizeGenericOoperand.
They not necessarily have extra EXEC operand.  I primarily take care of the copies introduced by the PHI legalization. 


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

https://reviews.llvm.org/D63731