[PATCH] D80434: [AMDGPU] Reject moving PHI to VALU if the only VGPR input originated from move immediate

[Alexander via Phabricator via llvm-commits]

alex-t added a comment.

In D80434#2051618 <https://reviews.llvm.org/D80434#2051618>, @rampitec wrote:

> In D80434#2051603 <https://reviews.llvm.org/D80434#2051603>, @alex-t wrote:
>
> > In D80434#2051602 <https://reviews.llvm.org/D80434#2051602>, @rampitec wrote:
> >
> > > You do not even need a loop to get PHI. A PHI might occur after an simple IF.
> > >  Nevertheless, patch retains an illegal copy which it is supposed to remove. The fact that it works in this testcase does not mean it will always work.
> > >  I would suggest to fold the immediate copy instead. I.e. you can replace COPY with an S_MOV_B32 (given it fits 32 bit of course).
> >
> >
> > The following code does it in one of the next iterations
> >
> >   // If we are just copying an immediate, we can replace the copy with
> >   // s_mov_b32.
> >   if (isSafeToFoldImmIntoCopy(&MI, DefMI, TII, SMovOp, Imm)) {
> >     MI.getOperand(1).ChangeToImmediate(Imm);
> >     MI.addImplicitDefUseOperands(MF);
> >     MI.setDesc(TII->get(SMovOp));
> >     break;
> >   }
> >   
> >   
>
>
> What if not isSafeToFoldImmIntoCopy?


Okay.  isSafeToFoldImmIntoCopy returns 'false' by one of the following:

  if (Copy->getOpcode() != AMDGPU::COPY)
     return false;
  
   if (!MoveImm->isMoveImmediate())
     return false;
  
   const MachineOperand *ImmOp =
       TII->getNamedOperand(*MoveImm, AMDGPU::OpName::src0);
   if (!ImmOp->isImm())
     return false;
  
   // FIXME: Handle copies with sub-regs.
   if (Copy->getOperand(0).getSubReg())
     return false;
  
   switch (MoveImm->getOpcode()) {
   default:
     return false;

The latter 2 conditions - subregs and exact move opcode - are not handled in my code.
So, what would you suggest:

1. guard with if (isSafeToFoldImmIntoCopy(...))
2. add subreg and exact opcode check to my 'if' ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80434