[PATCH] D52494: [InstCombine] Handle vector compares in foldGEPIcmp(), take 2

[Jesper Antonsson via Phabricator via llvm-commits]

JesperAntonsson created this revision.
JesperAntonsson added reviewers: lebedev.ri, majnemer, spatel.

This is a continuation of the fix for PR34627 "InstCombine assertion at vector gep/icmp folding". (I just realized bugpoint had fuzzed the original test for me, so I had fixed another trigger of the same assert in adjacent code in InstCombine.)

This patch avoids optimizing an icmp (to look only at the base pointers) when the resulting icmp would have a different type.

The patch adds a testcase and also cleans up and shrinks the pre-existing test for the adjacent assert trigger.


Repository:
  rL LLVM

https://reviews.llvm.org/D52494

Files:
  lib/Transforms/InstCombine/InstCombineCompares.cpp
  test/Transforms/InstCombine/pr38984.ll


Index: test/Transforms/InstCombine/pr38984.ll
===================================================================
--- test/Transforms/InstCombine/pr38984.ll
+++ test/Transforms/InstCombine/pr38984.ll
@@ -2,24 +2,40 @@
 ; RUN: opt < %s -instcombine -S | FileCheck %s
 target datalayout = "p:16:16"
 
- at offsets = external dso_local global [4 x i16], align 1
+ at a = external global [21 x i16], align 1
+ at offsets = external global [4 x i16], align 1
 
-define void @PR38984() {
-; CHECK-LABEL: @PR38984(
+; The "same gep" optimization should work with vector icmp.
+define <4 x i1> @PR38984_1() {
+; CHECK-LABEL: @PR38984_1(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    ret void
+; CHECK-NEXT:    ret <4 x i1> <i1 true, i1 true, i1 true, i1 true>
 ;
 entry:
   %0 = load i16, i16* getelementptr ([4 x i16], [4 x i16]* @offsets, i16 0, i16 undef), align 1
   %1 = insertelement <4 x i16> undef, i16 %0, i32 3
-  %2 = sub <4 x i16> zeroinitializer, %1
-  %3 = sext <4 x i16> %2 to <4 x i32>
-  %4 = getelementptr inbounds i64, i64* null, <4 x i32> %3
-  %5 = ptrtoint <4 x i64*> %4 to <4 x i32>
-  %6 = getelementptr inbounds i64, i64* null, <4 x i16> %2
-  %7 = ptrtoint <4 x i64*> %6 to <4 x i32>
-  %8 = icmp eq <4 x i32> %5, %7
-  %9 = select <4 x i1> %8, <4 x i16> zeroinitializer, <4 x i16> <i16 1, i16 1, i16 1, i16 1>
-  %10 = sext <4 x i16> %9 to <4 x i32>
-  ret void
+  %2 = getelementptr i32, i32* null, <4 x i16> %1
+  %3 = getelementptr i32, i32* null, <4 x i16> %1
+  %4 = icmp eq <4 x i32*> %2, %3
+  ret <4 x i1> %4
+}
+
+; The "compare base pointers" optimization should not kick in for vector icmp.
+define <4 x i1> @PR38984_2() {
+; CHECK-LABEL: @PR38984_2(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TMP0:%.*]] = load i16, i16* getelementptr ([4 x i16], [4 x i16]* @offsets, i16 0, i16 undef), align 2
+; CHECK-NEXT:    [[TMP1:%.*]] = insertelement <4 x i16> undef, i16 [[TMP0]], i32 3
+; CHECK-NEXT:    [[TMP2:%.*]] = getelementptr i16, i16* getelementptr inbounds ([21 x i16], [21 x i16]* @a, i16 1, i16 0), <4 x i16> [[TMP1]]
+; CHECK-NEXT:    [[TMP3:%.*]] = getelementptr i16, i16* null, <4 x i16> [[TMP1]]
+; CHECK-NEXT:    [[TMP4:%.*]] = icmp eq <4 x i16*> [[TMP2]], [[TMP3]]
+; CHECK-NEXT:    ret <4 x i1> [[TMP4]]
+;
+entry:
+  %0 = load i16, i16* getelementptr ([4 x i16], [4 x i16]* @offsets, i16 0, i16 undef)
+  %1 = insertelement <4 x i16> undef, i16 %0, i32 3
+  %2 = getelementptr i16, i16* getelementptr ([21 x i16], [21 x i16]* @a, i64 1, i32 0), <4 x i16> %1
+  %3 = getelementptr i16, i16* null, <4 x i16> %1
+  %4 = icmp eq <4 x i16*> %2, %3
+  ret <4 x i1> %4
 }
Index: lib/Transforms/InstCombine/InstCombineCompares.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -908,8 +908,19 @@
             break;
           }
 
+      // If we're going to replace the ICmp, we must end up with the same type.
+      const auto& CompatibleAsICmp = [](Value* Src, Value* Dst) -> bool {
+        if (Src->getType()->isVectorTy() != Dst->getType()->isVectorTy())
+          return false;
+        if (Src->getType()->isVectorTy() &&
+            Src->getType()->getVectorNumElements() !=
+            Dst->getType()->getVectorNumElements())
+          return false;
+        return true;
+      };
+
       // If all indices are the same, just compare the base pointers.
-      if (IndicesTheSame)
+      if (IndicesTheSame && CompatibleAsICmp(&I, PtrBase))
         return new ICmpInst(Cond, GEPLHS->getOperand(0), GEPRHS->getOperand(0));
 
       // If we're comparing GEPs with two base pointers that only differ in type


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D52494.166883.patch
Type: text/x-patch
Size: 3678 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180925/de0a34b3/attachment.bin>