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

[qcolombet]

Yes it does!

Thanks,
-Quentin
> On May 19, 2015, at 10:56 AM, Matthias Braun <matze at braunis.de> wrote:
> 
> I hope r237706 addresses the concerns.
> 
> - Matthias
> 
>> On May 19, 2015, at 10:04 AM, Quentin Colombet <qcolombet at apple.com <mailto:qcolombet at apple.com>> wrote:
>> 
>> Ping!
>> 
>>> On Apr 23, 2015, at 4:35 PM, Quentin Colombet <qcolombet at apple.com <mailto:qcolombet at apple.com>> wrote:
>>> 
>>> Hi Matthias,
>>> 
>>> Nice catch!
>>> A small nitpick.
>>> 
>>>> On Apr 23, 2015, at 4:24 PM, Matthias Braun <matze at braunis.de <mailto: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 <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 <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 <mailto:llvm-commits at cs.uiuc.edu>
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150519/a3bf61e9/attachment.html>