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

[Quentin Colombet]

Ping!

> On Apr 23, 2015, at 4:35 PM, Quentin Colombet <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> 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

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