[llvm] r321165 - [ARM GlobalISel] Fix assertion in RegBankSelect

Diana Picus via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 20 03:27:10 PST 2017


Author: rovka
Date: Wed Dec 20 03:27:10 2017
New Revision: 321165

URL: http://llvm.org/viewvc/llvm-project?rev=321165&view=rev
Log:
[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.

Modified:
    llvm/trunk/lib/Target/ARM/ARMInstructionSelector.cpp
    llvm/trunk/lib/Target/ARM/ARMRegisterBankInfo.cpp
    llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-instruction-select.mir
    llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-regbankselect.mir

Modified: llvm/trunk/lib/Target/ARM/ARMInstructionSelector.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMInstructionSelector.cpp?rev=321165&r1=321164&r2=321165&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMInstructionSelector.cpp (original)
+++ llvm/trunk/lib/Target/ARM/ARMInstructionSelector.cpp Wed Dec 20 03:27:10 2017
@@ -741,6 +741,31 @@ bool ARMInstructionSelector::select(Mach
     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;

Modified: llvm/trunk/lib/Target/ARM/ARMRegisterBankInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMRegisterBankInfo.cpp?rev=321165&r1=321164&r2=321165&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMRegisterBankInfo.cpp (original)
+++ llvm/trunk/lib/Target/ARM/ARMRegisterBankInfo.cpp Wed Dec 20 03:27:10 2017
@@ -226,12 +226,28 @@ ARMRegisterBankInfo::getInstrMapping(con
   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());

Modified: llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-instruction-select.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-instruction-select.mir?rev=321165&r1=321164&r2=321165&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-instruction-select.mir (original)
+++ llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-instruction-select.mir Wed Dec 20 03:27:10 2017
@@ -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

Modified: llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-regbankselect.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-regbankselect.mir?rev=321165&r1=321164&r2=321165&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-regbankselect.mir (original)
+++ llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-regbankselect.mir Wed Dec 20 03:27:10 2017
@@ -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




More information about the llvm-commits mailing list