[PATCH] D71660: [ValueTracking] isKnownNonZero() should take non-null-ness assumptions into consideration (PR43267)

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D71660#1789759 <https://reviews.llvm.org/D71660#1789759>, @lebedev.ri wrote:

> In D71660#1789726 <https://reviews.llvm.org/D71660#1789726>, @nikic wrote:
>
> > Have you considered integrating this in `isKnownNonNullFromDominatingCondition()` instead? It already iterates over all users of `V != 0` style conditions and could easily consider assumes in addition to branches.
> >  That may be more efficient than handling it separately.
>
>
> I'm not sure why it would be better than relying on the infrastructure specifically designed to handling assumptions?
>  It isn't obvious to me which approach is more performant, without benchmarks either approach may be much worse than the other one.


It's a question of which existing infrastructure you want to reuse ;) `isKnownNonNullFromDominatingCondition()` already handles many other scenarios involving dominating conditions and assume-like operations, but misses a check for the actual assumes.

In particular https://github.com/llvm-mirror/llvm/blob/2c4ca6832fa6b306ee6a7010bfb80a3f2596f824/lib/Analysis/ValueTracking.cpp#L1965-L1968 already deals with `llvm.experimental.guard()`. Adding support for assumes should be as simple as replacing `isGuard()` with `isGuard() || isAssume()` there.

Does that make sense?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71660