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

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added a comment.

In D88276#2295622 <https://reviews.llvm.org/D88276#2295622>, @nikic wrote:

> 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.)
>
> Edit: Current compile-time results for reference: https://llvm-compile-time-tracker.com/compare.php?from=f161e84c10b6eb2255345ebfaaa2bbadb4b0fe2a&to=541eefb28e624a99b22408b4322720bc6cd6972f&stat=instructions We should be able to do better than that.

Well, I can restrict the depth for phi nodes only to one level but it seems it causes the strange correlation to max depth. I tend to think that it would be better to increase depth to number of phi operands than restrict it to one level. In this case we can treat max depth as number of invocation of function. So it would be linear.

Please also note that right before phi handling we have a switch handling. It does not use one level depth as well as it does not modify the context instruction while select is actually can be considered as equivalent of phi node. Should be switch fixed as well?


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

https://reviews.llvm.org/D88276