Revert "[InstCombine] Support gep nuw in icmp folds"

[vitalybuka]

Reverts #118472

Breaks profile tests on i386 https://lab.llvm.org/buildbot/#/builders/66/builds/7009

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Vitaly Buka (vitalybuka)

Changes

Reverts llvm/llvm-project#118472

---

Full diff: https://github.com/llvm/llvm-project/pull/118698.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp (+9-27)
• (modified) llvm/test/Transforms/InstCombine/icmp-gep.ll (-75)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index b486c5b0b6fad0..56391d320e8b37 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -689,32 +689,13 @@ Instruction *InstCombinerImpl::foldGEPICmp(GEPOperator *GEPLHS, Value *RHS,
   if (!isa<GetElementPtrInst>(RHS))
     RHS = RHS->stripPointerCasts();
 
-  auto CanFold = [Cond](GEPNoWrapFlags NW) {
-    if (ICmpInst::isEquality(Cond))
-      return true;
-
-    // Unsigned predicates can be folded if the GEPs have *any* nowrap flags.
-    assert(ICmpInst::isUnsigned(Cond));
-    return NW != GEPNoWrapFlags::none();
-  };
-
-  auto NewICmp = [Cond](GEPNoWrapFlags NW, Value *Op1, Value *Op2) {
-    if (!NW.hasNoUnsignedWrap()) {
-      // Convert signed to unsigned comparison.
-      return new ICmpInst(ICmpInst::getSignedPredicate(Cond), Op1, Op2);
-    }
-
-    auto *I = new ICmpInst(Cond, Op1, Op2);
-    I->setSameSign(NW.hasNoUnsignedSignedWrap());
-    return I;
-  };
-
   Value *PtrBase = GEPLHS->getOperand(0);
-  if (PtrBase == RHS && CanFold(GEPLHS->getNoWrapFlags())) {
+  if (PtrBase == RHS &&
+      (GEPLHS->hasNoUnsignedSignedWrap() || ICmpInst::isEquality(Cond))) {
     // ((gep Ptr, OFFSET) cmp Ptr)   ---> (OFFSET cmp 0).
     Value *Offset = EmitGEPOffset(GEPLHS);
-    return NewICmp(GEPLHS->getNoWrapFlags(), Offset,
-                   Constant::getNullValue(Offset->getType()));
+    return new ICmpInst(ICmpInst::getSignedPredicate(Cond), Offset,
+                        Constant::getNullValue(Offset->getType()));
   }
 
   if (GEPLHS->isInBounds() && ICmpInst::isEquality(Cond) &&
@@ -832,18 +813,19 @@ Instruction *InstCombinerImpl::foldGEPICmp(GEPOperator *GEPLHS, Value *RHS,
         return replaceInstUsesWith(I, // No comparison is needed here.
           ConstantInt::get(I.getType(), ICmpInst::isTrueWhenEqual(Cond)));
 
-      else if (NumDifferences == 1 && CanFold(NW)) {
+      else if (NumDifferences == 1 && NW.hasNoUnsignedSignedWrap()) {
         Value *LHSV = GEPLHS->getOperand(DiffOperand);
         Value *RHSV = GEPRHS->getOperand(DiffOperand);
-        return NewICmp(NW, LHSV, RHSV);
+        // Make sure we do a signed comparison here.
+        return new ICmpInst(ICmpInst::getSignedPredicate(Cond), LHSV, RHSV);
       }
     }
 
-    if (CanFold(NW)) {
+    if (NW.hasNoUnsignedSignedWrap() || CmpInst::isEquality(Cond)) {
       // ((gep Ptr, OFFSET1) cmp (gep Ptr, OFFSET2)  --->  (OFFSET1 cmp OFFSET2)
       Value *L = EmitGEPOffset(GEPLHS, /*RewriteGEP=*/true);
       Value *R = EmitGEPOffset(GEPRHS, /*RewriteGEP=*/true);
-      return NewICmp(NW, L, R);
+      return new ICmpInst(ICmpInst::getSignedPredicate(Cond), L, R);
     }
   }
 
diff --git a/llvm/test/Transforms/InstCombine/icmp-gep.ll b/llvm/test/Transforms/InstCombine/icmp-gep.ll
index 1bc000cd6ebf1e..1545d034b2ac31 100644
--- a/llvm/test/Transforms/InstCombine/icmp-gep.ll
+++ b/llvm/test/Transforms/InstCombine/icmp-gep.ll
@@ -143,44 +143,6 @@ define i1 @ult_base_nusw(ptr %x, i64 %y) {
   ret i1 %r
 }
 
-define i1 @ugt_base_nuw(ptr %x, i64 %y) {
-; CHECK-LABEL: @ugt_base_nuw(
-; CHECK-NEXT:    [[R:%.*]] = icmp ne i64 [[Y:%.*]], 0
-; CHECK-NEXT:    ret i1 [[R]]
-;
-  %g = getelementptr nuw i8, ptr %x, i64 %y
-  %r = icmp ugt ptr %g, %x
-  ret i1 %r
-}
-
-define i1 @ugt_base_nusw_nuw(ptr %x, i64 %y) {
-; CHECK-LABEL: @ugt_base_nusw_nuw(
-; CHECK-NEXT:    [[R:%.*]] = icmp ne i64 [[Y:%.*]], 0
-; CHECK-NEXT:    ret i1 [[R]]
-;
-  %g = getelementptr nusw nuw i8, ptr %x, i64 %y
-  %r = icmp ugt ptr %g, %x
-  ret i1 %r
-}
-
-define i1 @uge_base_nuw(ptr %x, i64 %y) {
-; CHECK-LABEL: @uge_base_nuw(
-; CHECK-NEXT:    ret i1 true
-;
-  %g = getelementptr nuw i8, ptr %x, i64 %y
-  %r = icmp uge ptr %g, %x
-  ret i1 %r
-}
-
-define i1 @uge_base_nusw_nuw(ptr %x, i64 %y) {
-; CHECK-LABEL: @uge_base_nusw_nuw(
-; CHECK-NEXT:    ret i1 true
-;
-  %g = getelementptr nusw nuw i8, ptr %x, i64 %y
-  %r = icmp uge ptr %g, %x
-  ret i1 %r
-}
-
 define i1 @ugt_base_inbounds_commute(i64 %y) {
 ; CHECK-LABEL: @ugt_base_inbounds_commute(
 ; CHECK-NEXT:    [[X:%.*]] = call ptr @getptr()
@@ -357,43 +319,6 @@ define i1 @test60_nusw_inbounds(ptr %foo, i64 %i, i64 %j) {
   ret i1 %cmp
 }
 
-define i1 @test60_nuw(ptr %foo, i64 %i, i64 %j) {
-; CHECK-LABEL: @test60_nuw(
-; CHECK-NEXT:    [[GEP1_IDX:%.*]] = shl nuw i64 [[I:%.*]], 2
-; CHECK-NEXT:    [[CMP:%.*]] = icmp ult i64 [[GEP1_IDX]], [[J:%.*]]
-; CHECK-NEXT:    ret i1 [[CMP]]
-;
-  %gep1 = getelementptr nuw i32, ptr %foo, i64 %i
-  %gep2 = getelementptr nuw i8, ptr %foo, i64 %j
-  %cmp = icmp ult ptr %gep1, %gep2
-  ret i1 %cmp
-}
-
-define i1 @test60_nusw_nuw(ptr %foo, i64 %i, i64 %j) {
-; CHECK-LABEL: @test60_nusw_nuw(
-; CHECK-NEXT:    [[GEP1_IDX:%.*]] = shl nuw nsw i64 [[I:%.*]], 2
-; CHECK-NEXT:    [[CMP:%.*]] = icmp samesign ult i64 [[GEP1_IDX]], [[J:%.*]]
-; CHECK-NEXT:    ret i1 [[CMP]]
-;
-  %gep1 = getelementptr nusw nuw i32, ptr %foo, i64 %i
-  %gep2 = getelementptr nusw nuw i8, ptr %foo, i64 %j
-  %cmp = icmp ult ptr %gep1, %gep2
-  ret i1 %cmp
-}
-
-define i1 @test60_nusw_nuw_mix(ptr %foo, i64 %i, i64 %j) {
-; CHECK-LABEL: @test60_nusw_nuw_mix(
-; CHECK-NEXT:    [[GEP1:%.*]] = getelementptr nuw i32, ptr [[FOO:%.*]], i64 [[I:%.*]]
-; CHECK-NEXT:    [[GEP2:%.*]] = getelementptr nusw i8, ptr [[FOO]], i64 [[J:%.*]]
-; CHECK-NEXT:    [[CMP:%.*]] = icmp ult ptr [[GEP1]], [[GEP2]]
-; CHECK-NEXT:    ret i1 [[CMP]]
-;
-  %gep1 = getelementptr nuw i32, ptr %foo, i64 %i
-  %gep2 = getelementptr nusw i8, ptr %foo, i64 %j
-  %cmp = icmp ult ptr %gep1, %gep2
-  ret i1 %cmp
-}
-
 define i1 @test_gep_ult_no_inbounds(ptr %foo, i64 %i, i64 %j) {
 ; CHECK-LABEL: @test_gep_ult_no_inbounds(
 ; CHECK-NEXT:    [[GEP1:%.*]] = getelementptr i32, ptr [[FOO:%.*]], i64 [[I:%.*]]

[vitalybuka]

Hm, local revert didn't help.
My bisect was probably wrong.

[vitalybuka]

Hm, local revert didn't help. My bisect was probably wrong.

Bisect is correct. My revert validation was not. Clean rebuild of my work checkout was required.