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

[Matthias Braun]

I hope r237706 addresses the concerns.

- Matthias

> On May 19, 2015, at 10:04 AM, Quentin Colombet <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
> 
> _______________________________________________
> llvm-commits mailing list
> 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/3cfb5928/attachment.html>