[PATCH] D123748: [ValueTracking] Added support to deduce PHI Nodes values being a power of 2

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:2173
+  if (const PHINode *PN = dyn_cast<PHINode>(V)) {
+    unsigned NewDepth = std::max(Depth, MaxAnalysisRecursionDepth - 1);
+    Query RecQ = Q;
----------------
huangjd wrote:
> spatel wrote:
> > Carrot wrote:
> > > Please add a comment here. I searched the source code for a while to understand its purpose.
> > This request was not answered. We are using max depth to avoid a compile-time explosion because we are recursing on every phi operand?
> Looks like it should be just Depth instead, will fix it. 
> 
> A similar appearance in line 2580 (isKnownNonZero)  may be a bug then? This limits the level of recursion to be 2, instead of MaxAnalysisRecursionDepth. 
> This request was not answered. We are using max depth to avoid a compile-time explosion because we are recursing on every phi operand?

Yes. The depth limit is based on the assumption that instructions have few operands over which we recurse, generally resulting in up to `2^Depth` recursive calls at most. For phis the branching factor may be arbitrarily high, resulting in `Operands^Depth` recursive calls.

The limit is not strictly necessary for the recurrence case though, and I don't think we apply it for recurrences in KnownBits/NonZero.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123748