Skip to content

Commit

Permalink
[ARM GlobalISel] Fix selection of pointer constants
Browse files Browse the repository at this point in the history
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.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@321793 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
rovka committed Jan 4, 2018
1 parent f494e85 commit 79a984f
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 11 deletions.
34 changes: 26 additions & 8 deletions lib/Target/ARM/ARMInstructionSelector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -670,14 +670,6 @@ bool ARMInstructionSelector::select(MachineInstr &I,
}

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;
Expand Down Expand Up @@ -788,6 +780,32 @@ bool ARMInstructionSelector::select(MachineInstr &I,
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();
Expand Down
25 changes: 22 additions & 3 deletions test/CodeGen/ARM/GlobalISel/arm-instruction-select.mir
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 79a984f

Please sign in to comment.