[PATCH] D19369: VirtRegMap.cpp should not just remove an identity copy which also has a impl-def of a super-reg

[Quentin Colombet via llvm-commits]

We could fix MachineInstr:: isIdentityCopy instead. Indeed, the next optimization that removes them will hit the same issue. Moreover, I do not think we are supposed to have implicit_def around after TwoAddrPass (not entirely sure about that though).

Q.
> On Apr 21, 2016, at 9:19 AM, Jonas Paulsson <paulsson at linux.vnet.ibm.com> wrote:
> 
> jonpa created this revision.
> jonpa added a reviewer: qcolombet.
> jonpa added a subscriber: llvm-commits.
> 
> VirtRegMap removes identity copies, but that leads to machine-verifier complaints if there is a def of a super reg as well.
> 
> I am not sure what the best fix is, but the patch attached seems to fix this by transforming the COPY to an IMPLICIT_DEF when there is a def of a super-reg. The patch assumes (arbitrarily) that there could only be one extra operand - not sure if this is always true...
> 
> I put this with a testcase on Bugzilla a while ago and would appreciate any review.
> https://llvm.org/bugs/show_bug.cgi?id=27156
> 
> 
> http://reviews.llvm.org/D19369
> 
> Files:
>  lib/CodeGen/VirtRegMap.cpp
> 
> Index: lib/CodeGen/VirtRegMap.cpp
> ===================================================================
> --- lib/CodeGen/VirtRegMap.cpp
> +++ lib/CodeGen/VirtRegMap.cpp
> @@ -441,10 +441,25 @@
>       if (MI->isIdentityCopy()) {
>         ++NumIdCopies;
>         DEBUG(dbgs() << "Deleting identity copy.\n");
> -        if (Indexes)
> -          Indexes->removeMachineInstrFromMaps(*MI);
> -        // It's safe to erase MI because MII has already been incremented.
> -        MI->eraseFromParent();
> +
> +        // A COPY which has an implicit def of a super register must
> +        // become an IMPLICIT_DEF of that register.
> +        if (MI->getNumOperands() > 2) {
> +          MachineOperand &MO = MI->getOperand(2);
> +          if (MO.isDef() && MO.isImplicit()) {
> +            MI->setDesc(TII->get(TargetOpcode::IMPLICIT_DEF));
> +            MI->RemoveOperand(0);
> +            MI->RemoveOperand(0);
> +            MI->getOperand(0).setImplicit(false);
> +            assert (MI->getNumOperands() == 1 && "Not a single super reg implicit def?");
> +          }
> +        }
> +        else {
> +          if (Indexes)
> +            Indexes->removeMachineInstrFromMaps(*MI);
> +          // It's safe to erase MI because MII has already been incremented.
> +          MI->eraseFromParent();
> +        }
>       }
>     }
>   }
> 
> 
> <D19369.54517.patch>