[PATCH] D89968: [SVE][InstCombine] Fix TypeSize warning in canReplaceGEPIdxWithZero

Joe Ellis via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 22 08:52:42 PDT 2020


joechrisellis created this revision.
joechrisellis added reviewers: peterwaller-arm, fpetrogalli, DavidTruby.
Herald added subscribers: llvm-commits, psnobl, hiraditya, tschuett.
Herald added a reviewer: efriedma.
Herald added a project: LLVM.
joechrisellis requested review of this revision.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89968

Files:
  llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  llvm/test/Transforms/InstCombine/gep-can-replace-gep-idx-with-zero-typesize.ll


Index: llvm/test/Transforms/InstCombine/gep-can-replace-gep-idx-with-zero-typesize.ll
===================================================================
--- /dev/null
+++ 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
+}
Index: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -840,8 +840,13 @@
     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();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D89968.299999.patch
Type: text/x-patch
Size: 2389 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201022/fcf8c185/attachment.bin>


More information about the llvm-commits mailing list