[InstSimplify] Remove incorrect icmp of gep fold (PR52429)

[nikic]

Summary

As described in https://bugs.llvm.org/show_bug.cgi?id=52429 this fold is incorrect, because inbounds only guarantees that the pointers don't wrap in the unsigned space, it is possible that the sign boundary is crossed by an object.

I'm dropping the fold entirely rather than adjusting it, because computePointerICmp() fully subsumes it (with correct predicate handling).

[RKSimon]

llvm/test/Transforms/InstCombine/icmp-custom-dl.ll

163

Maybe add comments explaining why we don't fold each of these?

[nikic]

Add comments

[nlopes]

LGTM, thanks!

llvm/lib/Analysis/InstructionSimplify.cpp

if (auto *CRHS = dyn_cast<PtrToIntOperator>(RHS))
       if (Q.DL.getTypeSizeInBits(CLHS->getPointerOperandType()) ==
               Q.DL.getTypeSizeInBits(CLHS->getType()) &&
           Q.DL.getTypeSizeInBits(CRHS->getPointerOperandType()) ==
               Q.DL.getTypeSizeInBits(CRHS->getType()))
         if (auto *C = computePointerICmp(Pred, CLHS->getPointerOperand(),
                                          CRHS->getPointerOperand(), Q))
           return C;
 
-  if (GetElementPtrInst *GLHS = dyn_cast<GetElementPtrInst>(LHS)) {
-    if (GEPOperator *GRHS = dyn_cast<GEPOperator>(RHS)) {
-      if (GLHS->getPointerOperand() == GRHS->getPointerOperand() &&
-          GLHS->hasAllConstantIndices() && GRHS->hasAllConstantIndices() &&
-          (ICmpInst::isEquality(Pred) ||
-           (GLHS->isInBounds() && GRHS->isInBounds() &&
-            Pred == ICmpInst::getSignedPredicate(Pred)))) {
-        // The bases are equal and the indices are constant.  Build a constant
-        // expression GEP with the same indices and a null base pointer to see
-        // what constant folding can make out of it.
-        Constant *Null = Constant::getNullValue(GLHS->getPointerOperandType());
-        SmallVector<Value *, 4> IndicesLHS(GLHS->indices());
-        Constant *NewLHS = ConstantExpr::getGetElementPtr(
-            GLHS->getSourceElementType(), Null, IndicesLHS);
-
-        SmallVector<Value *, 4> IndicesRHS(GRHS->indices());
-        Constant *NewRHS = ConstantExpr::getGetElementPtr(
-            GLHS->getSourceElementType(), Null, IndicesRHS);
-        Constant *NewICmp = ConstantExpr::getICmp(Pred, NewLHS, NewRHS);
-        return ConstantFoldConstant(NewICmp, Q.DL);
-      }
-    }
-  }
-
   // If the comparison is with the result of a select instruction, check whether
   // comparing with either branch of the select always yields the same value.
   if (isa<SelectInst>(LHS) || isa<SelectInst>(RHS))
     if (Value *V = ThreadCmpOverSelect(Pred, LHS, RHS, Q, MaxRecurse))
       return V;
 
   // If the comparison is with the result of a phi instruction, check whether
   // doing the compare with each incoming phi value yields a common result.

llvm/test/Transforms/InstCombine/icmp-custom-dl.ll

;
   %gep1 = getelementptr i32, i32 addrspace(1)* %bit, i16 %i
   %gep2 = getelementptr i8, i8 addrspace(1)* %foo, i16 %j
   %cast1 = bitcast i32 addrspace(1)* %gep1 to i8 addrspace(1)*
   %cmp = icmp ult i8 addrspace(1)* %cast1, %gep2
   ret i1 %cmp
 ; Don't transform non-inbounds GEPs.
 }
 
+; Negative test: GEP inbounds may cross sign boundary.
 define i1 @test62(i8* %a) {
 ; CHECK-LABEL: @test62(
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    [[ARRAYIDX1:%.*]] = getelementptr inbounds i8, i8* [[A:%.*]], i32 1
+; CHECK-NEXT:    [[ARRAYIDX2:%.*]] = getelementptr inbounds i8, i8* [[A]], i32 10
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i8* [[ARRAYIDX1]], [[ARRAYIDX2]]
+; CHECK-NEXT:    ret i1 [[CMP]]
 ;
   %arrayidx1 = getelementptr inbounds i8, i8* %a, i64 1
   %arrayidx2 = getelementptr inbounds i8, i8* %a, i64 10
   %cmp = icmp slt i8* %arrayidx1, %arrayidx2
   ret i1 %cmp
 }
 
 define i1 @test62_as1(i8 addrspace(1)* %a) {
 ; CHECK-LABEL: @test62_as1(
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    [[ARRAYIDX1:%.*]] = getelementptr inbounds i8, i8 addrspace(1)* [[A:%.*]], i16 1
+; CHECK-NEXT:    [[ARRAYIDX2:%.*]] = getelementptr inbounds i8, i8 addrspace(1)* [[A]], i16 10
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i8 addrspace(1)* [[ARRAYIDX1]], [[ARRAYIDX2]]
+; CHECK-NEXT:    ret i1 [[CMP]]
 ;
   %arrayidx1 = getelementptr inbounds i8, i8 addrspace(1)* %a, i64 1
   %arrayidx2 = getelementptr inbounds i8, i8 addrspace(1)* %a, i64 10
   %cmp = icmp slt i8 addrspace(1)* %arrayidx1, %arrayidx2
   ret i1 %cmp
 }
 
 

llvm/test/Transforms/InstCombine/icmp.ll

 ; CHECK-NEXT:    ret void
 ;
   %cast = bitcast <1 x i64> <i64 36029346783166592> to i64
   %call = call i32 @test58_d( i64 %cast)
   ret void
 }
 declare i32 @test58_d(i64)
 
+; Negative test: GEP inbounds may cross sign boundary.
 define i1 @test62(i8* %a) {
 ; CHECK-LABEL: @test62(
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    [[ARRAYIDX1:%.*]] = getelementptr inbounds i8, i8* [[A:%.*]], i64 1
+; CHECK-NEXT:    [[ARRAYIDX2:%.*]] = getelementptr inbounds i8, i8* [[A]], i64 10
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i8* [[ARRAYIDX1]], [[ARRAYIDX2]]
+; CHECK-NEXT:    ret i1 [[CMP]]
 ;
   %arrayidx1 = getelementptr inbounds i8, i8* %a, i64 1
   %arrayidx2 = getelementptr inbounds i8, i8* %a, i64 10
   %cmp = icmp slt i8* %arrayidx1, %arrayidx2
   ret i1 %cmp
 }
 
 define i1 @test62_as1(i8 addrspace(1)* %a) {
 ; CHECK-LABEL: @test62_as1(
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    [[ARRAYIDX1:%.*]] = getelementptr inbounds i8, i8 addrspace(1)* [[A:%.*]], i16 1
+; CHECK-NEXT:    [[ARRAYIDX2:%.*]] = getelementptr inbounds i8, i8 addrspace(1)* [[A]], i16 10
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i8 addrspace(1)* [[ARRAYIDX1]], [[ARRAYIDX2]]
+; CHECK-NEXT:    ret i1 [[CMP]]
 ;
   %arrayidx1 = getelementptr inbounds i8, i8 addrspace(1)* %a, i64 1
   %arrayidx2 = getelementptr inbounds i8, i8 addrspace(1)* %a, i64 10
   %cmp = icmp slt i8 addrspace(1)* %arrayidx1, %arrayidx2
   ret i1 %cmp
 }
 
 define i1 @test63(i8 %a, i32 %b) {

llvm/test/Transforms/InstSimplify/compare.ll

;
   %b = getelementptr %gept, %gept* %x, i64 0, i32 1
   %equal = icmp eq i32* %a, %b
   ret i1 %equal
 }
 
 define i1 @gep7(%gept* %x) {
 ; CHECK-LABEL: @gep7(
 ; CHECK-NEXT:    [[A:%.*]] = getelementptr [[GEPT:%.*]], %gept* [[X:%.*]], i64 0, i32 0
-; CHECK-NEXT:    [[EQUAL:%.*]] = icmp eq i32* [[A]], getelementptr (%gept, %gept* @gepz, i32 0, i32 0)
+; CHECK-NEXT:    [[EQUAL:%.*]] = icmp eq i32* [[A]], getelementptr ([[GEPT]], %gept* @gepz, i32 0, i32 0)
 ; CHECK-NEXT:    ret i1 [[EQUAL]]
 ;
   %a = getelementptr %gept, %gept* %x, i64 0, i32 0
   %b = getelementptr %gept, %gept* @gepz, i64 0, i32 0
   %equal = icmp eq i32* %a, %b
   ret i1 %equal
 }
 
;
   %gep1 = getelementptr inbounds i32, i32* %alloca, i32 1
   %pti1 = ptrtoint i32* %gep1 to i32
   %gep2 = getelementptr inbounds [4 x i8], [4 x i8]* %bc, i32 0, i32 1
   %pti2 = ptrtoint i8* %gep2 to i32
   %cmp = icmp ugt i32 %pti1, %pti2
   ret i1 %cmp
 }
 
+; Negative test: GEP inbounds may cross sign boundary.
 define i1 @gep_same_base_constant_indices(i8* %a) {
 ; CHECK-LABEL: @gep_same_base_constant_indices(
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    [[ARRAYIDX1:%.*]] = getelementptr inbounds i8, i8* [[A:%.*]], i64 1
+; CHECK-NEXT:    [[ARRAYIDX2:%.*]] = getelementptr inbounds i8, i8* [[A]], i64 10
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i8* [[ARRAYIDX1]], [[ARRAYIDX2]]
+; CHECK-NEXT:    ret i1 [[CMP]]
 ;
   %arrayidx1 = getelementptr inbounds i8, i8* %a, i64 1
   %arrayidx2 = getelementptr inbounds i8, i8* %a, i64 10
   %cmp = icmp slt i8* %arrayidx1, %arrayidx2
   ret i1 %cmp
 }
 
 define i1 @zext(i32 %x) {