FastISel: Fix incorrectly using getPointerTy #110465

arsenm · 2024-09-30T08:23:15Z

This was using the default address space instead of the
correct one.

Fixes #56055

This was using the default address space instead of the correct one. Fixes #56055

arsenm · 2024-09-30T08:23:32Z

FastISel: Fix incorrectly using getPointerTy #110465 👈
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @arsenm and the rest of your teammates on Graphite

llvmbot · 2024-09-30T08:24:26Z

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-x86

Author: Matt Arsenault (arsenm)

Changes

This was using the default address space instead of the
correct one.

Fixes #56055

Full diff: https://github.com/llvm/llvm-project/pull/110465.diff

4 Files Affected:

(modified) llvm/include/llvm/CodeGen/FastISel.h (+1-1)
(modified) llvm/lib/CodeGen/SelectionDAG/FastISel.cpp (+4-4)
(modified) llvm/lib/Target/X86/X86FastISel.cpp (+3-1)
(added) llvm/test/CodeGen/X86/issue56055.ll (+81)

diff --git a/llvm/include/llvm/CodeGen/FastISel.h b/llvm/include/llvm/CodeGen/FastISel.h
index 3cbc35400181dd..f3c4cc8d0511d4 100644
--- a/llvm/include/llvm/CodeGen/FastISel.h
+++ b/llvm/include/llvm/CodeGen/FastISel.h
@@ -275,7 +275,7 @@ class FastISel {
 
   /// This is a wrapper around getRegForValue that also takes care of
   /// truncating or sign-extending the given getelementptr index value.
-  Register getRegForGEPIndex(const Value *Idx);
+  Register getRegForGEPIndex(MVT PtrVT, const Value *Idx);
 
   /// We're checking to see if we can fold \p LI into \p FoldInst. Note
   /// that we could have a sequence where multiple LLVM IR instructions are
diff --git a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
index 162af2d9d708a9..a4b0db90abb9fe 100644
--- a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -381,14 +381,13 @@ void FastISel::updateValueMap(const Value *I, Register Reg, unsigned NumRegs) {
   }
 }
 
-Register FastISel::getRegForGEPIndex(const Value *Idx) {
+Register FastISel::getRegForGEPIndex(MVT PtrVT, const Value *Idx) {
   Register IdxN = getRegForValue(Idx);
   if (!IdxN)
     // Unhandled operand. Halt "fast" selection and bail.
     return Register();
 
   // If the index is smaller or larger than intptr_t, truncate or extend it.
-  MVT PtrVT = TLI.getPointerTy(DL);
   EVT IdxVT = EVT::getEVT(Idx->getType(), /*HandleUnknown=*/false);
   if (IdxVT.bitsLT(PtrVT)) {
     IdxN = fastEmit_r(IdxVT.getSimpleVT(), PtrVT, ISD::SIGN_EXTEND, IdxN);
@@ -544,7 +543,8 @@ bool FastISel::selectGetElementPtr(const User *I) {
   uint64_t TotalOffs = 0;
   // FIXME: What's a good SWAG number for MaxOffs?
   uint64_t MaxOffs = 2048;
-  MVT VT = TLI.getPointerTy(DL);
+  MVT VT = TLI.getValueType(DL, I->getType()).getSimpleVT();
+
   for (gep_type_iterator GTI = gep_type_begin(I), E = gep_type_end(I);
        GTI != E; ++GTI) {
     const Value *Idx = GTI.getOperand();
@@ -585,7 +585,7 @@ bool FastISel::selectGetElementPtr(const User *I) {
 
       // N = N + Idx * ElementSize;
       uint64_t ElementSize = GTI.getSequentialElementStride(DL);
-      Register IdxN = getRegForGEPIndex(Idx);
+      Register IdxN = getRegForGEPIndex(VT, Idx);
       if (!IdxN) // Unhandled operand. Halt "fast" selection and bail.
         return false;
 
diff --git a/llvm/lib/Target/X86/X86FastISel.cpp b/llvm/lib/Target/X86/X86FastISel.cpp
index c6518a8e4363ec..4bf660b5e234ae 100644
--- a/llvm/lib/Target/X86/X86FastISel.cpp
+++ b/llvm/lib/Target/X86/X86FastISel.cpp
@@ -902,6 +902,8 @@ bool X86FastISel::X86SelectAddress(const Value *V, X86AddressMode &AM) {
     uint64_t Disp = (int32_t)AM.Disp;
     unsigned IndexReg = AM.IndexReg;
     unsigned Scale = AM.Scale;
+    MVT PtrVT = TLI.getValueType(DL, U->getType()).getSimpleVT();
+
     gep_type_iterator GTI = gep_type_begin(U);
     // Iterate through the indices, folding what we can. Constants can be
     // folded, and one dynamic index can be handled, if the scale is supported.
@@ -937,7 +939,7 @@ bool X86FastISel::X86SelectAddress(const Value *V, X86AddressMode &AM) {
             (S == 1 || S == 2 || S == 4 || S == 8)) {
           // Scaled-index addressing.
           Scale = S;
-          IndexReg = getRegForGEPIndex(Op);
+          IndexReg = getRegForGEPIndex(PtrVT, Op);
           if (IndexReg == 0)
             return false;
           break;
diff --git a/llvm/test/CodeGen/X86/issue56055.ll b/llvm/test/CodeGen/X86/issue56055.ll
new file mode 100644
index 00000000000000..27eaf13e3b00be
--- /dev/null
+++ b/llvm/test/CodeGen/X86/issue56055.ll
@@ -0,0 +1,81 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -fast-isel < %s | FileCheck -check-prefixes=CHECK,FASTISEL %s
+; RUN: llc < %s | FileCheck -check-prefixes=CHECK,SDAG %s
+
+target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-windows-msvc"
+
+define void @issue56055(ptr addrspace(270) %ptr, ptr %out) {
+; CHECK-LABEL: issue56055:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    addl $2, %ecx
+; CHECK-NEXT:    movl %ecx, (%rdx)
+; CHECK-NEXT:    retq
+  %add.ptr = getelementptr inbounds i8, ptr addrspace(270) %ptr, i32 2
+  store ptr addrspace(270) %add.ptr, ptr %out
+  ret void
+}
+
+define void @issue56055_vector(<2 x ptr addrspace(270)> %ptr, ptr %out) {
+; CHECK-LABEL: issue56055_vector:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    movdqa (%rcx), %xmm0
+; CHECK-NEXT:    paddd __xmm@00000000000000000000000200000002(%rip), %xmm0
+; CHECK-NEXT:    movq %xmm0, (%rdx)
+; CHECK-NEXT:    retq
+  %add.ptr = getelementptr inbounds i8, <2 x ptr addrspace(270)> %ptr, <2 x i32> <i32 2, i32 2>
+  store <2 x ptr addrspace(270)> %add.ptr, ptr %out
+  ret void
+}
+
+define void @issue56055_small_idx(ptr addrspace(270) %ptr, ptr %out, i16 %idx) {
+; CHECK-LABEL: issue56055_small_idx:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    movswl %r8w, %eax
+; CHECK-NEXT:    addl %ecx, %eax
+; CHECK-NEXT:    movl %eax, (%rdx)
+; CHECK-NEXT:    retq
+  %add.ptr = getelementptr inbounds i8, ptr addrspace(270) %ptr, i16 %idx
+  store ptr addrspace(270) %add.ptr, ptr %out
+  ret void
+}
+
+define void @issue56055_small_idx_vector(<2 x ptr addrspace(270)> %ptr, ptr %out, <2 x i16> %idx) {
+; CHECK-LABEL: issue56055_small_idx_vector:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    pshuflw {{.*#+}} xmm0 = mem[0,0,2,1,4,5,6,7]
+; CHECK-NEXT:    psrad $16, %xmm0
+; CHECK-NEXT:    paddd (%rcx), %xmm0
+; CHECK-NEXT:    movq %xmm0, (%rdx)
+; CHECK-NEXT:    retq
+  %add.ptr = getelementptr inbounds i8, <2 x ptr addrspace(270)> %ptr, <2 x i16> %idx
+  store <2 x ptr addrspace(270)> %add.ptr, ptr %out
+  ret void
+}
+
+define void @issue56055_large_idx(ptr addrspace(270) %ptr, ptr %out, i64 %idx) {
+; CHECK-LABEL: issue56055_large_idx:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    addl %ecx, %r8d
+; CHECK-NEXT:    movl %r8d, (%rdx)
+; CHECK-NEXT:    retq
+  %add.ptr = getelementptr inbounds i8, ptr addrspace(270) %ptr, i64 %idx
+  store ptr addrspace(270) %add.ptr, ptr %out
+  ret void
+}
+
+define void @issue56055_large_idx_vector(<2 x ptr addrspace(270)> %ptr, ptr %out, <2 x i64> %idx) {
+; CHECK-LABEL: issue56055_large_idx_vector:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    pshufd {{.*#+}} xmm0 = mem[0,2,2,3]
+; CHECK-NEXT:    paddd (%rcx), %xmm0
+; CHECK-NEXT:    movq %xmm0, (%rdx)
+; CHECK-NEXT:    retq
+  %add.ptr = getelementptr inbounds i8, <2 x ptr addrspace(270)> %ptr, <2 x i64> %idx
+  store <2 x ptr addrspace(270)> %add.ptr, ptr %out
+  ret void
+}
+
+;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
+; FASTISEL: {{.*}}
+; SDAG: {{.*}}

nikic

LGTM

arsenm · 2024-09-30T09:44:52Z

/cherry-pick

llvmbot · 2024-09-30T09:49:48Z

Failed to create pull request for issue110465 https://github.com/llvm/llvm-project/actions/runs/11103577248

arsenm · 2024-09-30T10:56:45Z

/cherry-pick 81ba95c

This was using the default address space instead of the correct one. Fixes llvm#56055 (cherry picked from commit 81ba95c)

llvmbot · 2024-09-30T11:01:52Z

/pull-request #110490

This was using the default address space instead of the correct one. Fixes llvm#56055 (cherry picked from commit 81ba95c)

This was using the default address space instead of the correct one. Fixes llvm#56055 Keep old method around for ABI compatibility on the release branch. (cherry picked from commit 81ba95c)

FastISel: Fix incorrectly using getPointerTy

e9ef3e4

This was using the default address space instead of the correct one. Fixes #56055

arsenm added backend:X86 llvm:SelectionDAG SelectionDAGISel as well labels Sep 30, 2024 — with Graphite App

arsenm requested review from amykhuang, e-kud and RKSimon September 30, 2024 08:24

arsenm marked this pull request as ready for review September 30, 2024 08:24

nikic approved these changes Sep 30, 2024

View reviewed changes

arsenm merged commit 81ba95c into main Sep 30, 2024
13 checks passed

arsenm deleted the users/arsenm/fast-isel-fix-issue56055 branch September 30, 2024 09:43

arsenm added this to the LLVM 19.X Release milestone Sep 30, 2024

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Sep 30, 2024

FastISel: Fix incorrectly using getPointerTy (llvm#110465)

2c0b211

This was using the default address space instead of the correct one. Fixes llvm#56055 (cherry picked from commit 81ba95c)

nikic pushed a commit to llvmbot/llvm-project that referenced this pull request Oct 1, 2024

FastISel: Fix incorrectly using getPointerTy (llvm#110465)

600ae3b

This was using the default address space instead of the correct one. Fixes llvm#56055 (cherry picked from commit 81ba95c)

thewtex mentioned this pull request Feb 10, 2025

llvmorg 19.1.5 libcxxabi pthread lib name #126605

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

FastISel: Fix incorrectly using getPointerTy #110465

FastISel: Fix incorrectly using getPointerTy #110465

Uh oh!

arsenm commented Sep 30, 2024

Uh oh!

arsenm commented Sep 30, 2024

Uh oh!

llvmbot commented Sep 30, 2024 •

edited

Loading

Uh oh!

nikic left a comment

Uh oh!

Uh oh!

arsenm commented Sep 30, 2024

Uh oh!

llvmbot commented Sep 30, 2024

Uh oh!

arsenm commented Sep 30, 2024

Uh oh!

llvmbot commented Sep 30, 2024

Uh oh!

Uh oh!

FastISel: Fix incorrectly using getPointerTy #110465

FastISel: Fix incorrectly using getPointerTy #110465

Uh oh!

Conversation

arsenm commented Sep 30, 2024

Uh oh!

arsenm commented Sep 30, 2024

Uh oh!

llvmbot commented Sep 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arsenm commented Sep 30, 2024

Uh oh!

llvmbot commented Sep 30, 2024

Uh oh!

arsenm commented Sep 30, 2024

Uh oh!

llvmbot commented Sep 30, 2024

Uh oh!

Uh oh!

llvmbot commented Sep 30, 2024 •

edited

Loading