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

diff --git a/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp b/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
index ecf8e1b..c68190b 100644 (file)
--- 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<LLT> AddrSpaces64 = {
+    GlobalPtr, ConstantPtr, FlatPtr
+  };
+
+  const std::initializer_list<LLT> 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 82e893b..76a9136 100644 (file)
--- 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 (file)
index 0000000..feb685f
--- /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
+...