[llvm] r315781 - [AArch64][RegisterBankInfo] Use the statically computed mappings for COPY

[Quentin Colombet via llvm-commits]

Re-committed as r315947

The mapping of G_BITCAST was not dealing with scalar type bigger than 64-bit
> On Oct 16, 2017, at 8:47 AM, Quentin Colombet via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Thanks Bruno.
> 
> I’ll take a look.
> 
>> On Oct 14, 2017, at 12:31 PM, Bruno Cardoso Lopes <bruno.cardoso at gmail.com> wrote:
>> 
>> Hi Quentin,
>> 
>> Reverted this in r315823 because it broke:
>> http://green.lab.llvm.org/green/job/Compiler_Verifiers_GlobalISEL/9882
>> 
>> On Fri, Oct 13, 2017 at 5:43 PM, Quentin Colombet via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>>> Author: qcolombet
>>> Date: Fri Oct 13 17:43:48 2017
>>> New Revision: 315781
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=315781&view=rev
>>> Log:
>>> [AArch64][RegisterBankInfo] Use the statically computed mappings for COPY
>>> 
>>> We use to resort on the generic implementation to get the mappings for
>>> COPYs. The generic implementation resorts on table lookup and
>>> dynamically allocated objects to get the valid mappings.
>>> 
>>> Given we already know how to map G_BITCAST and have the static mappings
>>> for them, use that code path for COPY as well. This is much more
>>> efficient.
>>> 
>>> Improve the compile time of RegBankSelect by up to 20%.
>>> 
>>> Note: When we eventually generate all the mappings via TableGen, we
>>> wouldn't have to do that dance to shave compile time. The intent of this
>>> change was to make sure that moving to static structure really pays off.
>>> 
>>> NFC.
>>> 
>>> Modified:
>>>   llvm/trunk/lib/Target/AArch64/AArch64RegisterBankInfo.cpp
>>> 
>>> Modified: llvm/trunk/lib/Target/AArch64/AArch64RegisterBankInfo.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64RegisterBankInfo.cpp?rev=315781&r1=315780&r2=315781&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/Target/AArch64/AArch64RegisterBankInfo.cpp (original)
>>> +++ llvm/trunk/lib/Target/AArch64/AArch64RegisterBankInfo.cpp Fri Oct 13 17:43:48 2017
>>> @@ -415,12 +415,10 @@ AArch64RegisterBankInfo::getSameKindOfOp
>>> const RegisterBankInfo::InstructionMapping &
>>> AArch64RegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
>>>  const unsigned Opc = MI.getOpcode();
>>> -  const MachineFunction &MF = *MI.getParent()->getParent();
>>> -  const MachineRegisterInfo &MRI = MF.getRegInfo();
>>> 
>>>  // Try the default logic for non-generic instructions that are either copies
>>>  // or already have some operands assigned to banks.
>>> -  if (!isPreISelGenericOpcode(Opc) ||
>>> +  if ((Opc != TargetOpcode::COPY && !isPreISelGenericOpcode(Opc)) ||
>>>      Opc == TargetOpcode::G_PHI) {
>>>    const RegisterBankInfo::InstructionMapping &Mapping =
>>>        getInstrMappingImpl(MI);
>>> @@ -428,6 +426,11 @@ AArch64RegisterBankInfo::getInstrMapping
>>>      return Mapping;
>>>  }
>>> 
>>> +  const MachineFunction &MF = *MI.getParent()->getParent();
>>> +  const MachineRegisterInfo &MRI = MF.getRegInfo();
>>> +  const TargetSubtargetInfo &STI = MF.getSubtarget();
>>> +  const TargetRegisterInfo &TRI = *STI.getRegisterInfo();
>>> +
>>>  switch (Opc) {
>>>    // G_{F|S|U}REM are not listed because they are not legal.
>>>    // Arithmetic ops.
>>> @@ -451,6 +454,30 @@ AArch64RegisterBankInfo::getInstrMapping
>>>  case TargetOpcode::G_FMUL:
>>>  case TargetOpcode::G_FDIV:
>>>    return getSameKindOfOperandsMapping(MI);
>>> +  case TargetOpcode::COPY: {
>>> +    unsigned DstReg = MI.getOperand(0).getReg();
>>> +    unsigned SrcReg = MI.getOperand(1).getReg();
>>> +    if (TargetRegisterInfo::isPhysicalRegister(DstReg) ||
>>> +        TargetRegisterInfo::isPhysicalRegister(SrcReg)) {
>>> +      const RegisterBank *DstRB = getRegBank(DstReg, MRI, TRI);
>>> +      const RegisterBank *SrcRB = getRegBank(SrcReg, MRI, TRI);
>>> +      if (!DstRB)
>>> +        DstRB = SrcRB;
>>> +      else if (!SrcRB)
>>> +        SrcRB = DstRB;
>>> +      // If both RB are null that means both registers are generic.
>>> +      // We shouldn't be here.
>>> +      assert(DstRB && SrcRB && "Both RegBank were nullptr");
>>> +      unsigned Size = getSizeInBits(DstReg, MRI, TRI);
>>> +      return getInstructionMapping(
>>> +          DefaultMappingID, copyCost(*DstRB, *SrcRB, Size),
>>> +          getCopyMapping(DstRB->getID(), SrcRB->getID(), Size),
>>> +          // We only care about the mapping of the destination.
>>> +          /*NumOperands*/ 1);
>>> +    }
>>> +    // Both registers are generic, use G_BITCAST.
>>> +    LLVM_FALLTHROUGH;
>>> +  }
>>>  case TargetOpcode::G_BITCAST: {
>>>    LLT DstTy = MRI.getType(MI.getOperand(0).getReg());
>>>    LLT SrcTy = MRI.getType(MI.getOperand(1).getReg());
>>> @@ -464,7 +491,8 @@ AArch64RegisterBankInfo::getInstrMapping
>>>    return getInstructionMapping(
>>>        DefaultMappingID, copyCost(DstRB, SrcRB, Size),
>>>        getCopyMapping(DstRB.getID(), SrcRB.getID(), Size),
>>> -        /*NumOperands*/ 2);
>>> +        // We only care about the mapping of the destination for COPY.
>>> +        /*NumOperands*/ Opc == TargetOpcode::G_BITCAST ? 2 : 1);
>>>  }
>>>  default:
>>>    break;
>>> 
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>> 
>> 
>> 
>> -- 
>> Bruno Cardoso Lopes
>> http://www.brunocardoso.cc
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171016/360360ae/attachment-0001.html>