From 9e5e868d95c25c1c5b09f315cc9f7f15885d61a2 Mon Sep 17 00:00:00 2001 From: Matt Arsenault Date: Thu, 14 Feb 2019 22:24:28 +0000 Subject: [PATCH] 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 --- .../lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp | 36 +++----- .../Target/AMDGPU/AMDGPURegisterBankInfo.cpp | 11 +-- .../AMDGPU/GlobalISel/regbankselect-gep.mir | 90 +++++++++++++++++++ 3 files changed, 105 insertions(+), 32 deletions(-) create mode 100644 llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-gep.mir diff --git a/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp b/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp index ecf8e1b732e9..c68190baf6f4 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp @@ -115,12 +115,12 @@ AMDGPULegalizerInfo::AMDGPULegalizerInfo(const GCNSubtarget &ST, const LLT CodePtr = FlatPtr; - const LLT AddrSpaces[] = { - GlobalPtr, - ConstantPtr, - LocalPtr, - FlatPtr, - PrivatePtr + const std::initializer_list AddrSpaces64 = { + GlobalPtr, ConstantPtr, FlatPtr + }; + + const std::initializer_list AddrSpaces32 = { + LocalPtr, PrivatePtr }; setAction({G_BRCOND, S1}, Legal); @@ -245,19 +245,11 @@ AMDGPULegalizerInfo::AMDGPULegalizerInfo(const GCNSubtarget &ST, .legalFor({S32, S64}) .scalarize(0); - for (LLT PtrTy : AddrSpaces) { - LLT IdxTy = LLT::scalar(PtrTy.getSizeInBits()); - setAction({G_GEP, PtrTy}, Legal); - setAction({G_GEP, 1, IdxTy}, Legal); - } - // FIXME: When RegBankSelect inserts copies, it will only create new registers - // with scalar types. This means we can end up with G_LOAD/G_STORE/G_GEP - // instruction with scalar types for their pointer operands. In assert builds, - // the instruction selector will assert if it sees a generic instruction which - // isn't legal, so we need to tell it that scalar types are legal for pointer - // operands - setAction({G_GEP, S64}, Legal); + getActionDefinitionsBuilder(G_GEP) + .legalForCartesianProduct(AddrSpaces64, {S64}) + .legalForCartesianProduct(AddrSpaces32, {S32}) + .scalarize(0); setAction({G_BLOCK_ADDR, CodePtr}, Legal); @@ -314,8 +306,8 @@ AMDGPULegalizerInfo::AMDGPULegalizerInfo(const GCNSubtarget &ST, getActionDefinitionsBuilder(G_INTTOPTR) // List the common cases - .legalForCartesianProduct({GlobalPtr, ConstantPtr, FlatPtr}, {S64}) - .legalForCartesianProduct({LocalPtr, PrivatePtr}, {S32}) + .legalForCartesianProduct(AddrSpaces64, {S64}) + .legalForCartesianProduct(AddrSpaces32, {S32}) .scalarize(0) // Accept any address space as long as the size matches .legalIf(sameSize(0, 1)) @@ -330,8 +322,8 @@ AMDGPULegalizerInfo::AMDGPULegalizerInfo(const GCNSubtarget &ST, getActionDefinitionsBuilder(G_PTRTOINT) // List the common cases - .legalForCartesianProduct({GlobalPtr, ConstantPtr, FlatPtr}, {S64}) - .legalForCartesianProduct({LocalPtr, PrivatePtr}, {S32}) + .legalForCartesianProduct(AddrSpaces64, {S64}) + .legalForCartesianProduct(AddrSpaces32, {S32}) .scalarize(0) // Accept any address space as long as the size matches .legalIf(sameSize(0, 1)) diff --git a/llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp b/llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp index 82e893bc7258..76a913625aee 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp @@ -605,6 +605,7 @@ AMDGPURegisterBankInfo::getInstrMapping(const MachineInstr &MI) const { LLVM_FALLTHROUGH; } + case AMDGPU::G_GEP: case AMDGPU::G_ADD: case AMDGPU::G_SUB: case AMDGPU::G_MUL: @@ -744,16 +745,6 @@ AMDGPURegisterBankInfo::getInstrMapping(const MachineInstr &MI) const { OpdsMapping[3] = AMDGPU::getValueMapping(AMDGPU::VGPRRegBankID, Size); break; } - case AMDGPU::G_GEP: { - for (unsigned i = 0, e = MI.getNumOperands(); i != e; ++i) { - if (!MI.getOperand(i).isReg()) - continue; - - unsigned Size = MRI.getType(MI.getOperand(i).getReg()).getSizeInBits(); - OpdsMapping[i] = AMDGPU::getValueMapping(AMDGPU::SGPRRegBankID, Size); - } - break; - } case AMDGPU::G_STORE: { assert(MI.getOperand(0).isReg()); unsigned Size = MRI.getType(MI.getOperand(0).getReg()).getSizeInBits(); diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-gep.mir b/llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-gep.mir new file mode 100644 index 000000000000..feb685ff8258 --- /dev/null +++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-gep.mir @@ -0,0 +1,90 @@ +# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py +# RUN: llc -march=amdgcn -run-pass=regbankselect %s -verify-machineinstrs -o - -regbankselect-fast | FileCheck %s +# RUN: llc -march=amdgcn -run-pass=regbankselect %s -verify-machineinstrs -o - -regbankselect-greedy | FileCheck %s + +--- +name: gep_p1_s_k +legalized: true + +body: | + bb.0: + liveins: $sgpr0_sgpr1 + + ; CHECK-LABEL: name: gep_p1_s_k + ; CHECK: [[COPY:%[0-9]+]]:sgpr(p1) = COPY $sgpr0_sgpr1 + ; CHECK: [[C:%[0-9]+]]:sgpr(s64) = G_CONSTANT i64 1 + ; CHECK: [[GEP:%[0-9]+]]:sgpr(p1) = G_GEP [[COPY]], [[C]](s64) + %0:_(p1) = COPY $sgpr0_sgpr1 + %1:_(s64) = G_CONSTANT i64 1 + %2:_(p1) = G_GEP %0, %1 +... + +--- +name: gep_p1_s_s +legalized: true + +body: | + bb.0: + liveins: $sgpr0_sgpr1, $sgpr2_sgpr3 + + ; CHECK-LABEL: name: gep_p1_s_s + ; CHECK: [[COPY:%[0-9]+]]:sgpr(p1) = COPY $sgpr0_sgpr1 + ; CHECK: [[COPY1:%[0-9]+]]:sgpr(s64) = COPY $sgpr2_sgpr3 + ; CHECK: [[GEP:%[0-9]+]]:sgpr(p1) = G_GEP [[COPY]], [[COPY1]](s64) + %0:_(p1) = COPY $sgpr0_sgpr1 + %1:_(s64) = COPY $sgpr2_sgpr3 + %2:_(p1) = G_GEP %0, %1 +... + +--- +name: gep_p1_v_k +legalized: true + +body: | + bb.0: + liveins: $vgpr0_vgpr1 + + ; CHECK-LABEL: name: gep_p1_v_k + ; CHECK: [[COPY:%[0-9]+]]:vgpr(p1) = COPY $vgpr0_vgpr1 + ; CHECK: [[C:%[0-9]+]]:sgpr(s64) = G_CONSTANT i64 1 + ; CHECK: [[COPY1:%[0-9]+]]:vgpr(s64) = COPY [[C]](s64) + ; CHECK: [[GEP:%[0-9]+]]:vgpr(p1) = G_GEP [[COPY]], [[COPY1]](s64) + %0:_(p1) = COPY $vgpr0_vgpr1 + %1:_(s64) = G_CONSTANT i64 1 + %2:_(p1) = G_GEP %0, %1 +... + +--- +name: gep_p1_v_s +legalized: true + +body: | + bb.0: + liveins: $vgpr0_vgpr1, $sgpr0_sgpr1 + + ; CHECK-LABEL: name: gep_p1_v_s + ; CHECK: [[COPY:%[0-9]+]]:vgpr(p1) = COPY $vgpr0_vgpr1 + ; CHECK: [[COPY1:%[0-9]+]]:sgpr(s64) = COPY $sgpr0_sgpr1 + ; CHECK: [[COPY2:%[0-9]+]]:vgpr(s64) = COPY [[COPY1]](s64) + ; CHECK: [[GEP:%[0-9]+]]:vgpr(p1) = G_GEP [[COPY]], [[COPY2]](s64) + %0:_(p1) = COPY $vgpr0_vgpr1 + %1:_(s64) = COPY $sgpr0_sgpr1 + %2:_(p1) = G_GEP %0, %1 +... + +--- +name: gep_p1_v_v +legalized: true + +body: | + bb.0: + liveins: $vgpr0_vgpr1, $vgpr2_vgpr3 + + ; CHECK-LABEL: name: gep_p1_v_v + ; CHECK: [[COPY:%[0-9]+]]:vgpr(p1) = COPY $vgpr0_vgpr1 + ; CHECK: [[COPY1:%[0-9]+]]:vgpr(s64) = COPY $vgpr2_vgpr3 + ; CHECK: [[GEP:%[0-9]+]]:vgpr(p1) = G_GEP [[COPY]], [[COPY1]](s64) + %0:_(p1) = COPY $vgpr0_vgpr1 + %1:_(s64) = COPY $vgpr2_vgpr3 + %2:_(p1) = G_GEP %0, %1 +...