[PATCH] D88276: [IsKnownNonZero] Handle the case with non-constant phi nodes

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

Thanks for the update, the new version should be correct. However, it can also be expensive, because we may loop through a phi multiple times until the depth limit is exhausted. I would suggest to follow the same implementation approach as computeKnownBits() does in https://github.com/llvm/llvm-project/blob/d2166076b882e38becf3657ea830ffd2b6a5695e/llvm/lib/Analysis/ValueTracking.cpp#L1574-L1606. Namely to perform the recursive call with MaxDepth - 1. (I would also suggest to handle your TODO right away, to have parity with the known bits code.)



================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:2567
+    bool AllNonZero = llvm::all_of(PN->operands(), [&](Value *V) {
+      return isKnownNonZero(V, DemandedElts, Depth, Q);
     });
----------------
aqjune wrote:
> nikic wrote:
> > It is necessary to adjust the context instruction in Q to be the terminator of the incoming edge here (see what computeKnownBits does for example).
> Does it mean that we need updates in a few other places as well (orthogonally from this patch)? e.g., https://github.com/llvm/llvm-project/blob/master/llvm/lib/Analysis/ValueTracking.cpp#L2970
Yes, looks like that one needs to be fixed as well.


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

https://reviews.llvm.org/D88276