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

[Quentin Colombet via llvm-commits]

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