[llvm] r315947 - Re-apply [AArch64][RegisterBankInfo] Use the statically computed mappings for COPY

Quentin Colombet via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 16 15:28:40 PDT 2017


Author: qcolombet
Date: Mon Oct 16 15:28:40 2017
New Revision: 315947

URL: http://llvm.org/viewvc/llvm-project?rev=315947&view=rev
Log:
Re-apply [AArch64][RegisterBankInfo] Use the statically computed mappings for COPY

This reverts commit r315823, thus re-applying r315781.

Also make sure we don't use G_BITCAST mapping for non-generic registers.
Non-generic registers don't have a type but do have a reg bank.
Something the COPY mapping now how to deal with but the G_BITCAST
mapping don't.

-- Original Commit Message --
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
    llvm/trunk/test/CodeGen/AArch64/GlobalISel/arm64-regbankselect.mir

Modified: llvm/trunk/lib/Target/AArch64/AArch64RegisterBankInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64RegisterBankInfo.cpp?rev=315947&r1=315946&r2=315947&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AArch64RegisterBankInfo.cpp (original)
+++ llvm/trunk/lib/Target/AArch64/AArch64RegisterBankInfo.cpp Mon Oct 16 15:28:40 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,33 @@ 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();
+    // Check if one of the register is not a generic register.
+    if ((TargetRegisterInfo::isPhysicalRegister(DstReg) ||
+         !MRI.getType(DstReg).isValid()) ||
+        (TargetRegisterInfo::isPhysicalRegister(SrcReg) ||
+         !MRI.getType(SrcReg).isValid())) {
+      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 +494,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;

Modified: llvm/trunk/test/CodeGen/AArch64/GlobalISel/arm64-regbankselect.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/GlobalISel/arm64-regbankselect.mir?rev=315947&r1=315946&r2=315947&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/GlobalISel/arm64-regbankselect.mir (original)
+++ llvm/trunk/test/CodeGen/AArch64/GlobalISel/arm64-regbankselect.mir Mon Oct 16 15:28:40 2017
@@ -67,6 +67,8 @@
   define void @bitcast_s64_gpr_fpr() { ret void }
   define void @bitcast_s64_fpr_gpr() { ret void }
   define void @bitcast_s128() { ret void }
+  define void @copy_s128() { ret void }
+  define void @copy_s128_from_load() { ret void }
 
   define i64 @greedyWithChainOfComputation(i64 %arg1, <2 x i32>* %addr) {
     %varg1 = bitcast i64 %arg1 to <2 x i32>
@@ -643,6 +645,69 @@ body:             |
 ...
 
 ---
+# CHECK-LABEL: name: copy_s128
+# This test checks that we issue the proper mapping
+# for copy of size > 64.
+# The mapping should be the same as G_BITCAST.
+name:            copy_s128
+legalized: true
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: _}
+  - { id: 1, class: _}
+  - { id: 2, class: _}
+  - { id: 3, class: _}
+  - { id: 4, class: _}
+# CHECK: registers:
+# CHECK:  - { id: 2, class: fpr, preferred-register: '' }
+# CHECK:  - { id: 3, class: fpr, preferred-register: '' }
+# CHECK:  - { id: 4, class: fpr, preferred-register: '' }
+# CHECK: %4(s128) = COPY %3(s128)
+# CHECK-NEXT: %2(<2 x s64>) = G_BITCAST %4(s128)
+body:             |
+  bb.1:
+    liveins: %x0, %x1
+    %0(s64) = COPY %x0
+    %1(s64) = COPY %x1
+    %3(s128) = G_MERGE_VALUES %0(s64), %1(s64)
+    %4(s128) = COPY %3(s128)
+    %2(<2 x s64>) = G_BITCAST %4(s128)
+    %q0 = COPY %2(<2 x s64>)
+    RET_ReallyLR implicit %q0
+
+...
+
+---
+# CHECK-LABEL: name: copy_s128_from_load
+# This test checks that we issue the proper mapping
+# for copy of size > 64 when the input is neither
+# a physcal register nor a generic register.
+# This used to crash when we moved to the statically
+# computed mapping, because we were assuming non-physregs
+# were generic registers and thus have a type, whereas
+# it is not necessarily the case.
+name:            copy_s128_from_load
+legalized: true
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: fpr128}
+  - { id: 1, class: _}
+# CHECK: registers:
+# CHECK:  - { id: 0, class: fpr128, preferred-register: '' }
+# CHECK:  - { id: 1, class: fpr, preferred-register: '' }
+# CHECK: %1(s128) = COPY %0
+body:             |
+  bb.1:
+    liveins: %x0
+    %0 = LDRQui killed %x0, 0
+    %1(s128) = COPY %0
+    %q0 = COPY %1(s128)
+    RET_ReallyLR implicit %q0
+
+...
+
+
+---
 # Make sure the greedy mode is able to take advantage of the
 # alternative mappings of G_LOAD to coalesce the whole chain
 # of computation on GPR.




More information about the llvm-commits mailing list