InstCombine: Don't transform a signed icmp of two GEPs into a signed compare of the indices.

This transformation is not safe in some pathological cases (signed icmp of pointers should be an
extremely rare thing, but it's valid IR!). Add an explanatory comment.

Kudos to Duncan for pointing out this edge case (and not giving up explaining it until I finally got it).

llvm-svn: 151055

lib/Transforms/InstCombine/InstCombineCompares.cpp

@@ -571,6 +571,14 @@ static Value *EvaluateGEPOffsetExpression(User *GEP, InstCombiner &IC) {
 Instruction *InstCombiner::FoldGEPICmp(GEPOperator *GEPLHS, Value *RHS,
                                        ICmpInst::Predicate Cond,
                                        Instruction &I) {
+  // Don't transform signed compares of GEPs into index compares. Even if the
+  // GEP is inbounds, the final add of the base pointer can have signed overflow
+  // and would change the result of the icmp.
+  // e.g. "&foo[0] <s &foo[1]" can't be folded to "true" because "foo" could be
+  // the minimum signed value for the pointer type.
+  if (ICmpInst::isSigned(Cond))
+    return 0;
+
   // Look through bitcasts.
   if (BitCastInst *BCI = dyn_cast<BitCastInst>(RHS))
     RHS = BCI->getOperand(0);

test/Transforms/InstCombine/icmp.ll

@@ -628,3 +628,14 @@ define i1 @test61(i8* %foo, i64 %i, i64 %j) {
 ; CHECK: icmp ult i8* %cast1, %gep2
 ; CHECK-NEXT: ret i1
 }
+
+define i1 @test62(i8* %a) {
+  %arrayidx1 = getelementptr inbounds i8* %a, i64 1
+  %arrayidx2 = getelementptr inbounds i8* %a, i64 10
+  %cmp = icmp slt i8* %arrayidx1, %arrayidx2
+  ret i1 %cmp
+; Don't turn a signed cmp of GEPs into an index compare.
+; CHECK: @test62
+; CHECK: %cmp = icmp slt i8* %arrayidx1, %arrayidx2
+; CHECK-NEXT: ret i1 %cmp
+}