[llvm] 0f83505 - [SVE][InstCombine] Fix TypeSize warning in canReplaceGEPIdxWithZero

[Joe Ellis via llvm-commits]

Author: Joe Ellis
Date: 2020-10-26T17:40:26Z
New Revision: 0f8350559325db35e0ee3423db5d29113e4eec12

URL: https://github.com/llvm/llvm-project/commit/0f8350559325db35e0ee3423db5d29113e4eec12
DIFF: https://github.com/llvm/llvm-project/commit/0f8350559325db35e0ee3423db5d29113e4eec12.diff

LOG: [SVE][InstCombine] Fix TypeSize warning in canReplaceGEPIdxWithZero

The warning would fire when calling canReplaceGEPIdxWithZero on a GEP
whose source element type is a scalable vector. The size of scalable
vector types is not known, so this optimization cannot be performed.

This patch fixes the issue by:

- bailing out early in this routine if the GEP instruction's source
  element type is a scalable vector.

- making use of getFixedSize -- this removes the dependency on the
  deprecated interface.

Reviewed By: fpetrogalli

Differential Revision: https://reviews.llvm.org/D89968

Added: 
    llvm/test/Transforms/InstCombine/gep-can-replace-gep-idx-with-zero-typesize.ll

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index e3b22aaefc9e..ac617ecd4fd1 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -840,12 +840,17 @@ static bool canReplaceGEPIdxWithZero(InstCombinerImpl &IC,
     return false;
 
   SmallVector<Value *, 4> Ops(GEPI->idx_begin(), GEPI->idx_begin() + Idx);
-  Type *AllocTy =
-    GetElementPtrInst::getIndexedType(GEPI->getSourceElementType(), Ops);
+  Type *SourceElementType = GEPI->getSourceElementType();
+  // Size information about scalable vectors is not available, so we cannot
+  // deduce whether indexing at n is undefined behaviour or not. Bail out.
+  if (isa<ScalableVectorType>(SourceElementType))
+    return false;
+
+  Type *AllocTy = GetElementPtrInst::getIndexedType(SourceElementType, Ops);
   if (!AllocTy || !AllocTy->isSized())
     return false;
   const DataLayout &DL = IC.getDataLayout();
-  uint64_t TyAllocSize = DL.getTypeAllocSize(AllocTy);
+  uint64_t TyAllocSize = DL.getTypeAllocSize(AllocTy).getFixedSize();
 
   // If there are more indices after the one we might replace with a zero, make
   // sure they're all non-negative. If any of them are negative, the overall

diff  --git a/llvm/test/Transforms/InstCombine/gep-can-replace-gep-idx-with-zero-typesize.ll b/llvm/test/Transforms/InstCombine/gep-can-replace-gep-idx-with-zero-typesize.ll
new file mode 100644
index 000000000000..6e8d0fd4e99c
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/gep-can-replace-gep-idx-with-zero-typesize.ll
@@ -0,0 +1,25 @@
+; RUN: opt -S -O2 -mtriple=aarch64-linux-gnu -mattr=+sve < %s 2>%t
+; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
+
+; This regression test is verifying that the optimization defined by
+; canReplaceGEPIdxWithZero, which replaces a GEP index with zero iff we can show
+; a value other than zero would cause undefined behaviour, does not throw a
+; 'assumption that TypeSize is not scalable' warning when the source element type
+; is a scalable vector.
+
+; If the source element is a scalable vector type, then we cannot deduce whether
+; or not indexing at a given index is undefined behaviour, because the size of
+; the vector is not known.
+
+; If this check fails please read test/CodeGen/AArch64/README for instructions
+; on how to resolve it.
+; WARN-NOT: warning: {{.*}}TypeSize is not scalable
+
+declare void @do_something(<vscale x 4 x i32> %x)
+
+define void @can_replace_gep_idx_with_zero_typesize(i64 %n, <vscale x 4 x i32>* %a, i64 %b) {
+  %idx = getelementptr <vscale x 4 x i32>, <vscale x 4 x i32>* %a, i64 %b
+  %tmp = load <vscale x 4 x i32>, <vscale x 4 x i32>* %idx
+  call void @do_something(<vscale x 4 x i32> %tmp)
+  ret void
+}