[llvm] r321793 - [ARM GlobalISel] Fix selection of pointer constants

[Diana Picus via llvm-commits]

Author: rovka
Date: Thu Jan  4 02:54:57 2018
New Revision: 321793

URL: http://llvm.org/viewvc/llvm-project?rev=321793&view=rev
Log:
[ARM GlobalISel] Fix selection of pointer constants

We used to handle G_CONSTANT with pointer type by forcing the type of
the result register to s32 and then letting TableGen handle it.
Unfortunately, setting the type only works for generic virtual
registers, that haven't yet been constrained to a register class (e.g.
those used only by a COPY later on). If the result register has already
been constrained as a use of a previously selected instruction, then
setting the type will assert.

It would be nice to be able to teach TableGen to select pointer
constants the same as integer constants, but since it's such an edge
case (at the moment the only pointer constant that we're generally
interested in is 0, and that is mostly used for comparisons and selects,
which are also not supported by TableGen) it's probably not worth the
effort right now. Instead, handle pointer constants with some trivial
handwritten code.

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

Modified: llvm/trunk/lib/Target/ARM/ARMInstructionSelector.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMInstructionSelector.cpp?rev=321793&r1=321792&r2=321793&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMInstructionSelector.cpp (original)
+++ llvm/trunk/lib/Target/ARM/ARMInstructionSelector.cpp Thu Jan  4 02:54:57 2018
@@ -670,14 +670,6 @@ bool ARMInstructionSelector::select(Mach
   }
 
   using namespace TargetOpcode;
-  if (I.getOpcode() == G_CONSTANT) {
-    // Pointer constants should be treated the same as 32-bit integer constants.
-    // Change the type and let TableGen handle it.
-    unsigned ResultReg = I.getOperand(0).getReg();
-    LLT Ty = MRI.getType(ResultReg);
-    if (Ty.isPointer())
-      MRI.setType(ResultReg, LLT::scalar(32));
-  }
 
   if (selectImpl(I, CoverageInfo))
     return true;
@@ -788,6 +780,32 @@ bool ARMInstructionSelector::select(Mach
     I.setDesc(TII.get(COPY));
     return selectCopy(I, TII, MRI, TRI, RBI);
   }
+  case G_CONSTANT: {
+    if (!MRI.getType(I.getOperand(0).getReg()).isPointer()) {
+      // Non-pointer constants should be handled by TableGen.
+      DEBUG(dbgs() << "Unsupported constant type\n");
+      return false;
+    }
+
+    auto &Val = I.getOperand(1);
+    if (Val.isCImm()) {
+      if (!Val.getCImm()->isZero()) {
+        DEBUG(dbgs() << "Unsupported pointer constant value\n");
+        return false;
+      }
+      Val.ChangeToImmediate(0);
+    } else {
+      assert(Val.isImm() && "Unexpected operand for G_CONSTANT");
+      if (Val.getImm() != 0) {
+        DEBUG(dbgs() << "Unsupported pointer constant value\n");
+        return false;
+      }
+    }
+
+    I.setDesc(TII.get(ARM::MOVi));
+    MIB.add(predOps(ARMCC::AL)).add(condCodeOp());
+    break;
+  }
   case G_INTTOPTR:
   case G_PTRTOINT: {
     auto SrcReg = I.getOperand(1).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=321793&r1=321792&r2=321793&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-instruction-select.mir (original)
+++ llvm/trunk/test/CodeGen/ARM/GlobalISel/arm-instruction-select.mir Thu Jan  4 02:54:57 2018
@@ -47,7 +47,8 @@
   define void @test_gep() { ret void }
   define void @test_constant_imm() { ret void }
   define void @test_constant_cimm() { ret void }
-  define void @test_pointer_constant() { ret void }
+  define void @test_pointer_constant_unconstrained() { ret void }
+  define void @test_pointer_constant_constrained() { ret void }
 
   define void @test_inttoptr_s32() { ret void }
   define void @test_ptrtoint_s32() { ret void }
@@ -1110,8 +1111,8 @@ body:             |
     BX_RET 14, %noreg, implicit %r0
 ...
 ---
-name:            test_pointer_constant
-# CHECK-LABEL: name: test_pointer_constant
+name:            test_pointer_constant_unconstrained
+# CHECK-LABEL: name: test_pointer_constant_unconstrained
 legalized:       true
 regBankSelected: true
 selected:        false
@@ -1123,10 +1124,28 @@ body:             |
     %0(p0) = G_CONSTANT i32 0
     ; CHECK: %[[C:[0-9]+]]:gpr = MOVi 0, 14, %noreg, %noreg
 
+    ; This leaves %0 unconstrained before the G_CONSTANT is selected.
     %r0 = COPY %0(p0)
     BX_RET 14, %noreg, implicit %r0
 ...
 ---
+name:            test_pointer_constant_constrained
+# CHECK-LABEL: name: test_pointer_constant_constrained
+legalized:       true
+regBankSelected: true
+selected:        false
+# CHECK: selected: true
+registers:
+  - { id: 0, class: gprb }
+body:             |
+  bb.0:
+    %0(p0) = G_CONSTANT i32 0
+    ; CHECK: %[[C:[0-9]+]]:gpr = MOVi 0, 14, %noreg, %noreg
+
+    ; This constrains %0 before the G_CONSTANT is selected.
+    G_STORE %0(p0), %0(p0) :: (store 4)
+...
+---
 name:            test_inttoptr_s32
 # CHECK-LABEL: name: test_inttoptr_s32
 legalized:       true