[ARM GlobalISel] Fix assertion in RegBankSelect

We get an assertion in RegBankSelect for code along the lines of
my_32_bit_int = my_64_bit_int, which tends to translate into a 64-bit
load, followed by a G_TRUNC, followed by a 32-bit store. This appears in
a couple of places in the test-suite.

At the moment, the legalizer doesn't distinguish between integer and
floating point scalars, so a 64-bit load will be marked as legal for
targets with VFP, and so will the rest of the sequence, leading to a
slightly bizarre G_TRUNC reaching RegBankSelect.

Since the current support for 64-bit integers is rather immature, this
patch works around the issue by explicitly handling this case in
RegBankSelect and InstructionSelect. In the future, we may want to
revisit this decision and make sure 64-bit integer loads are narrowed
before reaching RegBankSelect.

llvm-svn: 321165

4 files changed, 98 insertions, 1 deletions

diff --git a/llvm/lib/Target/ARM/ARMInstructionSelector.cpp b/llvm/lib/Target/ARM/ARMInstructionSelector.cpp
index 6bbeae2e115..de2f79df49b 100644
--- a/llvm/lib/Target/ARM/ARMInstructionSelector.cpp
+++ b/llvm/lib/Target/ARM/ARMInstructionSelector.cpp
@@ -741,6 +741,31 @@ bool ARMInstructionSelector::select(MachineInstr &I,
     const auto &SrcRegBank = *RBI.getRegBank(SrcReg, MRI, TRI);
     const auto &DstRegBank = *RBI.getRegBank(DstReg, MRI, TRI);
 
+    if (SrcRegBank.getID() == ARM::FPRRegBankID) {
+      // This should only happen in the obscure case where we have put a 64-bit
+      // integer into a D register. Get it out of there and keep only the
+      // interesting part.
+      assert(I.getOpcode() == G_TRUNC && "Unsupported operand for G_ANYEXT");
+      assert(DstRegBank.getID() == ARM::GPRRegBankID &&
+             "Unsupported combination of register banks");
+      assert(MRI.getType(SrcReg).getSizeInBits() == 64 && "Unsupported size");
+      assert(MRI.getType(DstReg).getSizeInBits() <= 32 && "Unsupported size");
+
+      unsigned IgnoredBits = MRI.createVirtualRegister(&ARM::GPRRegClass);
+      auto InsertBefore = std::next(I.getIterator());
+      auto MovI =
+          BuildMI(MBB, InsertBefore, I.getDebugLoc(), TII.get(ARM::VMOVRRD))
+              .addDef(DstReg)
+              .addDef(IgnoredBits)
+              .addUse(SrcReg)
+              .add(predOps(ARMCC::AL));
+      if (!constrainSelectedInstRegOperands(*MovI, TII, TRI, RBI))
+        return false;
+
+      MIB->eraseFromParent();
+      return true;
+    }
+
     if (SrcRegBank.getID() != DstRegBank.getID()) {
       DEBUG(dbgs() << "G_TRUNC/G_ANYEXT operands on different register banks\n");
       return false;
diff --git a/llvm/lib/Target/ARM/ARMRegisterBankInfo.cpp b/llvm/lib/Target/ARM/ARMRegisterBankInfo.cpp
index b32bfd44954..32d1d57d326 100644
--- a/llvm/lib/Target/ARM/ARMRegisterBankInfo.cpp
+++ b/llvm/lib/Target/ARM/ARMRegisterBankInfo.cpp
@@ -226,12 +226,28 @@ ARMRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
   case G_SEXT:
   case G_ZEXT:
   case G_ANYEXT:
-  case G_TRUNC:
   case G_GEP:
     // FIXME: We're abusing the fact that everything lives in a GPR for now; in
     // the real world we would use different mappings.
     OperandsMapping = &ARM::ValueMappings[ARM::GPR3OpsIdx];
     break;
+  case G_TRUNC: {
+    // In some cases we may end up with a G_TRUNC from a 64-bit value to a
+    // 32-bit value. This isn't a real floating point trunc (that would be a
+    // G_FPTRUNC). Instead it is an integer trunc in disguise, which can appear
+    // because the legalizer doesn't distinguish between integer and floating
+    // point values so it may leave some 64-bit integers un-narrowed. Until we
+    // have a more principled solution that doesn't let such things sneak all
+    // the way to this point, just map the source to a DPR and the destination
+    // to a GPR.
+    LLT LargeTy = MRI.getType(MI.getOperand(1).getReg());
+    OperandsMapping =
+        LargeTy.getSizeInBits() <= 32
+            ? &ARM::ValueMappings[ARM::GPR3OpsIdx]
+            : getOperandsMapping({&ARM::ValueMappings[ARM::GPR3OpsIdx],
+                                  &ARM::ValueMappings[ARM::DPR3OpsIdx]});
+    break;
+  }
   case G_LOAD:
   case G_STORE: {
     LLT Ty = MRI.getType(MI.getOperand(0).getReg());
diff --git a/llvm/test/CodeGen/ARM/GlobalISel/arm-instruction-select.mir b/llvm/test/CodeGen/ARM/GlobalISel/arm-instruction-select.mir
index 7c2666e3680..53efa2cf0ff 100644
--- a/llvm/test/CodeGen/ARM/GlobalISel/arm-instruction-select.mir
+++ b/llvm/test/CodeGen/ARM/GlobalISel/arm-instruction-select.mir
@@ -6,6 +6,7 @@
   define void @test_trunc_and_zext_s16() { ret void }
   define void @test_trunc_and_anyext_s8() { ret void }
   define void @test_trunc_and_anyext_s16() { ret void }
+  define void @test_trunc_s64() #0 { ret void }
 
   define void @test_add_s32() { ret void }
   define void @test_add_fold_imm_s32() { ret void }
@@ -241,6 +242,36 @@ body:             |
     ; CHECK: BX_RET 14, %noreg, implicit %r0
 ...
 ---
+name:            test_trunc_s64
+# CHECK-LABEL: name: test_trunc_s64
+legalized:       true
+regBankSelected: true
+selected:        false
+# CHECK: selected: true
+registers:
+  - { id: 0, class: fprb }
+  - { id: 1, class: gprb }
+  - { id: 2, class: gprb }
+body:             |
+  bb.0:
+    liveins: %r0, %d0
+
+    %0(s64) = COPY %d0
+    ; CHECK: [[VREG:%[0-9]+]]:dpr = COPY %d0
+
+    %2(p0) = COPY %r0
+    ; CHECK: [[PTR:%[0-9]+]]:gpr = COPY %r0
+
+    %1(s32) = G_TRUNC %0(s64)
+    ; CHECK: [[VREGTRUNC:%[0-9]+]]:gpr, [[UNINTERESTING:%[0-9]+]]:gpr = VMOVRRD [[VREG]]
+
+    G_STORE %1(s32), %2 :: (store 4)
+    ; CHECK: STRi12 [[VREGTRUNC]], [[PTR]], 0, 14, %noreg
+
+    BX_RET 14, %noreg
+    ; CHECK: BX_RET 14, %noreg
+...
+---
 name:            test_add_s32
 # CHECK-LABEL: name: test_add_s32
 legalized:       true
diff --git a/llvm/test/CodeGen/ARM/GlobalISel/arm-regbankselect.mir b/llvm/test/CodeGen/ARM/GlobalISel/arm-regbankselect.mir
index 044740e33a2..1ce4b8b98ec 100644
--- a/llvm/test/CodeGen/ARM/GlobalISel/arm-regbankselect.mir
+++ b/llvm/test/CodeGen/ARM/GlobalISel/arm-regbankselect.mir
@@ -31,6 +31,7 @@
   define void @test_anyext_s16_32() { ret void }
 
   define void @test_trunc_s32_16() { ret void }
+  define void @test_trunc_s64_32() #0 { ret void }
 
   define void @test_icmp_eq_s32() { ret void }
   define void @test_fcmp_one_s32() #0 { ret void }
@@ -584,6 +585,30 @@ body:             |
     BX_RET 14, %noreg
 ...
 ---
+name:            test_trunc_s64_32
+# CHECK-LABEL: name: test_trunc_s64_32
+legalized:       true
+regBankSelected: false
+selected:        false
+# CHECK: registers:
+# CHECK: - { id: 0, class: fprb, preferred-register: '' }
+# CHECK: - { id: 1, class: gprb, preferred-register: '' }
+# CHECK: - { id: 2, class: gprb, preferred-register: '' }
+registers:
+  - { id: 0, class: _ }
+  - { id: 1, class: _ }
+  - { id: 2, class: _ }
+body:             |
+  bb.0:
+    liveins: %r0, %d0
+
+    %0(s64) = COPY %d0
+    %2(p0) = COPY %r0
+    %1(s32) = G_TRUNC %0(s64)
+    G_STORE %1(s32), %2 :: (store 4)
+    BX_RET 14, %noreg
+...
+---
 name:            test_icmp_eq_s32
 # CHECK-LABEL: name: test_icmp_eq_s32
 legalized:       true