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

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).

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@151055 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Transforms/InstCombine/InstCombineCompares.cpp b/lib/Transforms/InstCombine/InstCombineCompares.cpp
index b62f6e204969ae594a3ae0f820700575ea240f3a..2f608b26acc58a6538128771800aed24bea5af20 100644 (file)
--- a/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/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);

diff --git a/test/Transforms/InstCombine/icmp.ll b/test/Transforms/InstCombine/icmp.ll
index 8c4871942cb35bd0592798a5d8b4161938114b25..dabb0f3adfe4c8cb3d5f5129edd901d83fad9e63 100644 (file)
--- a/test/Transforms/InstCombine/icmp.ll
+++ b/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
+}