[llvm] r235668 - RegisterCoalescer: Avoid unnecessary register class widening for some rematerializations

[Quentin Colombet]

Hi Matthias,

Nice catch!
A small nitpick.

> On Apr 23, 2015, at 4:24 PM, Matthias Braun <matze at braunis.de> wrote:
> 
> Author: matze
> Date: Thu Apr 23 18:24:36 2015
> New Revision: 235668
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=235668&view=rev
> Log:
> RegisterCoalescer: Avoid unnecessary register class widening for some rematerializations
> 
> I couldn't provide a testcase as none of the public targets has wide
> register classes with alot of subregisters and at the same time an
> instruction which "ReMaterializable" and "AsCheapAsAMove" (could
> probably be added for R600).
> 
> Modified:
>    llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp
> 
> Modified: llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp?rev=235668&r1=235667&r2=235668&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp (original)
> +++ llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp Thu Apr 23 18:24:36 2015
> @@ -194,7 +194,7 @@ namespace {
> 
>     /// If the source of a copy is defined by a
>     /// trivial computation, replace the copy by rematerialize the definition.
> -    bool reMaterializeTrivialDef(CoalescerPair &CP, MachineInstr *CopyMI,
> +    bool reMaterializeTrivialDef(const CoalescerPair &CP, MachineInstr *CopyMI,
>                                  bool &IsDefCopy);
> 
>     /// Return true if a copy involving a physreg should be joined.
> @@ -851,7 +851,7 @@ static bool definesFullReg(const Machine
>   return false;
> }
> 
> -bool RegisterCoalescer::reMaterializeTrivialDef(CoalescerPair &CP,
> +bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
>                                                 MachineInstr *CopyMI,
>                                                 bool &IsDefCopy) {
>   IsDefCopy = false;
> @@ -929,6 +929,29 @@ bool RegisterCoalescer::reMaterializeTri
>   TII->reMaterialize(*MBB, MII, DstReg, SrcIdx, DefMI, *TRI);
>   MachineInstr *NewMI = std::prev(MII);
> 
> +  // A situation like the following:
> +  //     %vreg0:subX = instr           ; DefMI
> +  //     %vregY      = copy %vreg:subX ; CopyMI

To make the comment (or code) more readable, please use the same name for the comment and the code for subX.

Cheers,
-Quentin

> +  // does not need subregisters/regclass widening after rematerialization, just
> +  // do:
> +  //     %vregY = instr
> +  const TargetRegisterClass *NewRC = CP.getNewRC();
> +  if (DstIdx != 0) {
> +    MachineOperand &DefMO = NewMI->getOperand(0);
> +    if (DefMO.getSubReg() == DstIdx) {
> +      assert(SrcIdx == 0 && CP.isFlipped()
> +             && "Shouldn't have SrcIdx+DstIdx at this point");
> +      const TargetRegisterClass *DstRC = MRI->getRegClass(DstReg);
> +      const TargetRegisterClass *CommonRC =
> +        TRI->getCommonSubClass(DefRC, DstRC);
> +      if (CommonRC != nullptr) {
> +        NewRC = CommonRC;
> +        DstIdx = 0;
> +        DefMO.setSubReg(0);
> +      }
> +    }
> +  }
> +
>   LIS->ReplaceMachineInstrInMaps(CopyMI, NewMI);
>   CopyMI->eraseFromParent();
>   ErasedInstrs.insert(CopyMI);
> @@ -948,7 +971,6 @@ bool RegisterCoalescer::reMaterializeTri
>   }
> 
>   if (TargetRegisterInfo::isVirtualRegister(DstReg)) {
> -    const TargetRegisterClass *NewRC = CP.getNewRC();
>     unsigned NewIdx = NewMI->getOperand(0).getSubReg();
> 
>     if (DefRC != nullptr) {
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits