[PATCH] D60504: NFC: Improve pattern matching in computeKnownBitsFromAssume.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen marked 2 inline comments as done.
sdesmalen added a comment.

We found a case where repeated calls into computeKnownBits(FromAssume) skyrocketed our build-times after we added a call to llvm.assume after vectorized loops to describe the range of a newly introduced induction variable, causing some builds to time-out.
Because the 'less then' or 'less then equal' case was way down this list in this function, it had to go through all the matching calls. Unfortunately I don't have any real numbers to back this up, because we did this work a while ago.



================
Comment at: lib/Analysis/ValueTracking.cpp:619-620
 
+    ICmpInst *Arg = dyn_cast<ICmpInst>(I->getArgOperand(0));
+    if (!Arg)
+      continue;
----------------
spatel wrote:
> IMO, it would make things a bit clearer to name this "Cmp" instead of recycling "Arg" as the name. Then use that name throughout the later code.
Yes, I agree that is clearer.


================
Comment at: lib/Analysis/ValueTracking.cpp:631
+      if (match(Arg, m_c_ICmp(Pred, m_V, m_Value(A))) &&
+          Pred == ICmpInst::ICMP_EQ && isValidAssumeForContext(I, Q.CxtI, Q.DT)) {
+        KnownBits RHSKnown(BitWidth);
----------------
spatel wrote:
> Here and later, please remove the duplicated check of the predicate value.
Good catch!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60504/new/

https://reviews.llvm.org/D60504