Skip to content

Commit 9e5e868

Browse files
arsenmarsenm
committed
AMDGPU/GlobalISel: Fix RegBankSelect for GEP.
This is basically a pointer typed add, so shouldn't be any different. This was assuming everything was an SGPR, which is not true. Also cleanup legality for GEP. I don't seem to be seeing the problem the hack marking s64 as a legal pointer type the comment mentions. llvm-svn: 354067
1 parent 871821f commit 9e5e868

File tree

3 files changed

+105
-32
lines changed

3 files changed

+105
-32
lines changed

‎llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,12 @@ AMDGPULegalizerInfo::AMDGPULegalizerInfo(const GCNSubtarget &ST,
115115

116116
const LLT CodePtr = FlatPtr;
117117

118-
const LLT AddrSpaces[] = {
119-
GlobalPtr,
120-
ConstantPtr,
121-
LocalPtr,
122-
FlatPtr,
123-
PrivatePtr
118+
const std::initializer_list<LLT> AddrSpaces64 = {
119+
GlobalPtr, ConstantPtr, FlatPtr
120+
};
121+
122+
const std::initializer_list<LLT> AddrSpaces32 = {
123+
LocalPtr, PrivatePtr
124124
};
125125

126126
setAction({G_BRCOND, S1}, Legal);
@@ -245,19 +245,11 @@ AMDGPULegalizerInfo::AMDGPULegalizerInfo(const GCNSubtarget &ST,
245245
.legalFor({S32, S64})
246246
.scalarize(0);
247247

248-
for (LLT PtrTy : AddrSpaces) {
249-
LLT IdxTy = LLT::scalar(PtrTy.getSizeInBits());
250-
setAction({G_GEP, PtrTy}, Legal);
251-
setAction({G_GEP, 1, IdxTy}, Legal);
252-
}
253248

254-
// FIXME: When RegBankSelect inserts copies, it will only create new registers
255-
// with scalar types. This means we can end up with G_LOAD/G_STORE/G_GEP
256-
// instruction with scalar types for their pointer operands. In assert builds,
257-
// the instruction selector will assert if it sees a generic instruction which
258-
// isn't legal, so we need to tell it that scalar types are legal for pointer
259-
// operands
260-
setAction({G_GEP, S64}, Legal);
249+
getActionDefinitionsBuilder(G_GEP)
250+
.legalForCartesianProduct(AddrSpaces64, {S64})
251+
.legalForCartesianProduct(AddrSpaces32, {S32})
252+
.scalarize(0);
261253

262254
setAction({G_BLOCK_ADDR, CodePtr}, Legal);
263255

@@ -314,8 +306,8 @@ AMDGPULegalizerInfo::AMDGPULegalizerInfo(const GCNSubtarget &ST,
314306

315307
getActionDefinitionsBuilder(G_INTTOPTR)
316308
// List the common cases
317-
.legalForCartesianProduct({GlobalPtr, ConstantPtr, FlatPtr}, {S64})
318-
.legalForCartesianProduct({LocalPtr, PrivatePtr}, {S32})
309+
.legalForCartesianProduct(AddrSpaces64, {S64})
310+
.legalForCartesianProduct(AddrSpaces32, {S32})
319311
.scalarize(0)
320312
// Accept any address space as long as the size matches
321313
.legalIf(sameSize(0, 1))
@@ -330,8 +322,8 @@ AMDGPULegalizerInfo::AMDGPULegalizerInfo(const GCNSubtarget &ST,
330322

331323
getActionDefinitionsBuilder(G_PTRTOINT)
332324
// List the common cases
333-
.legalForCartesianProduct({GlobalPtr, ConstantPtr, FlatPtr}, {S64})
334-
.legalForCartesianProduct({LocalPtr, PrivatePtr}, {S32})
325+
.legalForCartesianProduct(AddrSpaces64, {S64})
326+
.legalForCartesianProduct(AddrSpaces32, {S32})
335327
.scalarize(0)
336328
// Accept any address space as long as the size matches
337329
.legalIf(sameSize(0, 1))

‎llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,7 @@ AMDGPURegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
605605
LLVM_FALLTHROUGH;
606606
}
607607

608+
case AMDGPU::G_GEP:
608609
case AMDGPU::G_ADD:
609610
case AMDGPU::G_SUB:
610611
case AMDGPU::G_MUL:
@@ -744,16 +745,6 @@ AMDGPURegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
744745
OpdsMapping[3] = AMDGPU::getValueMapping(AMDGPU::VGPRRegBankID, Size);
745746
break;
746747
}
747-
case AMDGPU::G_GEP: {
748-
for (unsigned i = 0, e = MI.getNumOperands(); i != e; ++i) {
749-
if (!MI.getOperand(i).isReg())
750-
continue;
751-
752-
unsigned Size = MRI.getType(MI.getOperand(i).getReg()).getSizeInBits();
753-
OpdsMapping[i] = AMDGPU::getValueMapping(AMDGPU::SGPRRegBankID, Size);
754-
}
755-
break;
756-
}
757748
case AMDGPU::G_STORE: {
758749
assert(MI.getOperand(0).isReg());
759750
unsigned Size = MRI.getType(MI.getOperand(0).getReg()).getSizeInBits();

‎llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-gep.mir

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
2+
# RUN: llc -march=amdgcn -run-pass=regbankselect %s -verify-machineinstrs -o - -regbankselect-fast | FileCheck %s
3+
# RUN: llc -march=amdgcn -run-pass=regbankselect %s -verify-machineinstrs -o - -regbankselect-greedy | FileCheck %s
4+
5+
---
6+
name: gep_p1_s_k
7+
legalized: true
8+
9+
body: |
10+
bb.0:
11+
liveins: $sgpr0_sgpr1
12+
13+
; CHECK-LABEL: name: gep_p1_s_k
14+
; CHECK: [[COPY:%[0-9]+]]:sgpr(p1) = COPY $sgpr0_sgpr1
15+
; CHECK: [[C:%[0-9]+]]:sgpr(s64) = G_CONSTANT i64 1
16+
; CHECK: [[GEP:%[0-9]+]]:sgpr(p1) = G_GEP [[COPY]], [[C]](s64)
17+
%0:_(p1) = COPY $sgpr0_sgpr1
18+
%1:_(s64) = G_CONSTANT i64 1
19+
%2:_(p1) = G_GEP %0, %1
20+
...
21+
22+
---
23+
name: gep_p1_s_s
24+
legalized: true
25+
26+
body: |
27+
bb.0:
28+
liveins: $sgpr0_sgpr1, $sgpr2_sgpr3
29+
30+
; CHECK-LABEL: name: gep_p1_s_s
31+
; CHECK: [[COPY:%[0-9]+]]:sgpr(p1) = COPY $sgpr0_sgpr1
32+
; CHECK: [[COPY1:%[0-9]+]]:sgpr(s64) = COPY $sgpr2_sgpr3
33+
; CHECK: [[GEP:%[0-9]+]]:sgpr(p1) = G_GEP [[COPY]], [[COPY1]](s64)
34+
%0:_(p1) = COPY $sgpr0_sgpr1
35+
%1:_(s64) = COPY $sgpr2_sgpr3
36+
%2:_(p1) = G_GEP %0, %1
37+
...
38+
39+
---
40+
name: gep_p1_v_k
41+
legalized: true
42+
43+
body: |
44+
bb.0:
45+
liveins: $vgpr0_vgpr1
46+
47+
; CHECK-LABEL: name: gep_p1_v_k
48+
; CHECK: [[COPY:%[0-9]+]]:vgpr(p1) = COPY $vgpr0_vgpr1
49+
; CHECK: [[C:%[0-9]+]]:sgpr(s64) = G_CONSTANT i64 1
50+
; CHECK: [[COPY1:%[0-9]+]]:vgpr(s64) = COPY [[C]](s64)
51+
; CHECK: [[GEP:%[0-9]+]]:vgpr(p1) = G_GEP [[COPY]], [[COPY1]](s64)
52+
%0:_(p1) = COPY $vgpr0_vgpr1
53+
%1:_(s64) = G_CONSTANT i64 1
54+
%2:_(p1) = G_GEP %0, %1
55+
...
56+
57+
---
58+
name: gep_p1_v_s
59+
legalized: true
60+
61+
body: |
62+
bb.0:
63+
liveins: $vgpr0_vgpr1, $sgpr0_sgpr1
64+
65+
; CHECK-LABEL: name: gep_p1_v_s
66+
; CHECK: [[COPY:%[0-9]+]]:vgpr(p1) = COPY $vgpr0_vgpr1
67+
; CHECK: [[COPY1:%[0-9]+]]:sgpr(s64) = COPY $sgpr0_sgpr1
68+
; CHECK: [[COPY2:%[0-9]+]]:vgpr(s64) = COPY [[COPY1]](s64)
69+
; CHECK: [[GEP:%[0-9]+]]:vgpr(p1) = G_GEP [[COPY]], [[COPY2]](s64)
70+
%0:_(p1) = COPY $vgpr0_vgpr1
71+
%1:_(s64) = COPY $sgpr0_sgpr1
72+
%2:_(p1) = G_GEP %0, %1
73+
...
74+
75+
---
76+
name: gep_p1_v_v
77+
legalized: true
78+
79+
body: |
80+
bb.0:
81+
liveins: $vgpr0_vgpr1, $vgpr2_vgpr3
82+
83+
; CHECK-LABEL: name: gep_p1_v_v
84+
; CHECK: [[COPY:%[0-9]+]]:vgpr(p1) = COPY $vgpr0_vgpr1
85+
; CHECK: [[COPY1:%[0-9]+]]:vgpr(s64) = COPY $vgpr2_vgpr3
86+
; CHECK: [[GEP:%[0-9]+]]:vgpr(p1) = G_GEP [[COPY]], [[COPY1]](s64)
87+
%0:_(p1) = COPY $vgpr0_vgpr1
88+
%1:_(s64) = COPY $vgpr2_vgpr3
89+
%2:_(p1) = G_GEP %0, %1
90+
...

0 commit comments

Comments
 (0)