[PATCH] D100408: [ValueTracking][InstSimplify] improve efficiency for detecting non-zero value

[Sanjay Patel via Phabricator via llvm-commits]

spatel created this revision.
spatel added reviewers: nikic, lebedev.ri, xbolva00.
Herald added subscribers: hiraditya, mcrosier.
spatel requested review of this revision.
Herald added a project: LLVM.

I was stepping through some of the callstacks in the example from D99759 <https://reviews.llvm.org/D99759> and noticed a potential compile-time improvement.
This is really 2 patches, but I've put them together to make it easier to see if it does anything to help real-world compile-time:

1. Improve the logic in isNonZeroRecurrence() to handle the case of a negative-stepping phi+add pair.
2. Limit a call from instsimplify to isKnownNonEqual() to avoid a pile of redundant analysis.

The first step is needed to avoid a regression - the test example shown here is caught by isKnownNonEqual() but missed by isNonZeroRecurrence() without this patch.

This makes a degenerate test based on PR49785 about 40x faster (25 sec -> 0.6 sec), but it does not address the larger question of how to limit computeKnownBitsFromAssume(). Ie, the original test there is still infinite-time for all practical purposes.


https://reviews.llvm.org/D100408

Files:
  llvm/lib/Analysis/InstructionSimplify.cpp
  llvm/lib/Analysis/ValueTracking.cpp
  llvm/unittests/Analysis/ValueTrackingTest.cpp


Index: llvm/unittests/Analysis/ValueTrackingTest.cpp
===================================================================
--- llvm/unittests/Analysis/ValueTrackingTest.cpp
+++ llvm/unittests/Analysis/ValueTrackingTest.cpp
@@ -1191,7 +1191,7 @@
   )");
   DataLayout DL = M->getDataLayout();
   AssumptionCache AC(*F);
-  EXPECT_FALSE(isKnownNonZero(A, DL, 0, &AC, CxtI));
+  EXPECT_TRUE(isKnownNonZero(A, DL, 0, &AC, CxtI));
 }
 
 TEST_F(ValueTrackingTest, KnownNonZeroFromDomCond) {
Index: llvm/lib/Analysis/ValueTracking.cpp
===================================================================
--- llvm/lib/Analysis/ValueTracking.cpp
+++ llvm/lib/Analysis/ValueTracking.cpp
@@ -2213,9 +2213,9 @@
   return true;
 }
 
+/// Try to detect a recurrence that monotonically increases/decreases from a
+/// non-zero starting value. These are common as induction variables.
 static bool isNonZeroRecurrence(const PHINode *PN) {
-  // Try and detect a recurrence that monotonically increases from a
-  // starting value, as these are common as induction variables.
   BinaryOperator *BO = nullptr;
   Value *Start = nullptr, *Step = nullptr;
   const APInt *StartC, *StepC;
@@ -2225,11 +2225,19 @@
 
   switch (BO->getOpcode()) {
   case Instruction::Add:
-    return match(Step, m_APInt(StepC)) &&
-           ((BO->hasNoUnsignedWrap() && !StartC->isNullValue() &&
-             !StepC->isNullValue()) ||
-            (BO->hasNoSignedWrap() && StartC->isStrictlyPositive() &&
-             StepC->isNonNegative()));
+    if (!match(Step, m_APInt(StepC)))
+      return false;
+    if (BO->hasNoUnsignedWrap()) {
+      if (!StartC->isNullValue() && !StepC->isNullValue())
+        return true;
+    }
+    if (BO->hasNoSignedWrap()) {
+      // Starting from non-zero and stepping away from zero can never wrap back
+      // to zero.
+      if (!StartC->isNullValue() && StartC->isNegative() == StepC->isNegative())
+        return true;
+    }
+    return false;
   case Instruction::Mul:
     return !StartC->isNullValue() && match(Step, m_APInt(StepC)) &&
            ((BO->hasNoUnsignedWrap() && !StepC->isNullValue()) ||
Index: llvm/lib/Analysis/InstructionSimplify.cpp
===================================================================
--- llvm/lib/Analysis/InstructionSimplify.cpp
+++ llvm/lib/Analysis/InstructionSimplify.cpp
@@ -3637,7 +3637,9 @@
   }
 
   // icmp eq|ne X, Y -> false|true if X != Y
-  if (ICmpInst::isEquality(Pred) &&
+  // This is potentially expensive, and we have already computedKnownBits for
+  // compares with 0 above here, so only try this for a non-zero compare.
+  if (ICmpInst::isEquality(Pred) && !match(RHS, m_Zero()) &&
       isKnownNonEqual(LHS, RHS, Q.DL, Q.AC, Q.CxtI, Q.DT, Q.IIQ.UseInstrInfo)) {
     return Pred == ICmpInst::ICMP_NE ? getTrue(ITy) : getFalse(ITy);
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D100408.337227.patch
Type: text/x-patch
Size: 2813 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210413/a9bee9c9/attachment.bin>