[PATCH] D71386: [AMDGPU] Remove unnecessary v_mov from a register to itself in WQM lowering.

[Michael Bedy via Phabricator via llvm-commits]

mjbedy marked 4 inline comments as done.
mjbedy added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIWholeQuadMode.cpp:877-878
+  for (MachineInstr *MI : LowerToCopyInstrs) {
+    for (unsigned i = MI->getNumExplicitOperands() - 1; i > 1; i--)
+      MI->RemoveOperand(i);
+
----------------
foad wrote:
> mjbedy wrote:
> > nhaehnle wrote:
> > > arsenm wrote:
> > > > These should only ever have been moves to begin with? Just remove the one exec operand?
> > > They were move-like: WQM, SOFT_WQM, V_SET_INACTIVE_xx. I think it makes sense though to just remove that one operand and maybe assert there are no other implicit uses.
> > Now that I look closer at this, I'm not even sure what this loop is doing (or the one in the case above that I modeled it on)? The exec operands are implicit so this doesn't touch them. This is what comes into the phase:
> > 
> > %12:vgpr_32 = WQM %10:vgpr_32, implicit $exec, implicit $exec
> > 
> > And this is what comes out:
> > 
> > %12:vgpr_32 = COPY %10:vgpr_32, implicit $exec, implicit $exec
> > 
> The loop removes any extra explicit operands after the first two, which will become the destination and source of the copy. Currently the SET_INACTIVE instructions have an extra explicit operand called "inactive".
> 
> Several passes add an implicit use of exec without checking whether there was one already, so I'm not surprised this pass does not bother to remove any implicit operands. You can see instructions with multiple "implicit $exec" operands in debug output and even in some of our autogenerated lit tests.
I fixed the code to only expect this one case, and added a test for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71386