[PATCH] D100573: [ValueTracking] don't recursively compute known bits using multiple llvm.assumes

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D100573#2692468 <https://reviews.llvm.org/D100573#2692468>, @spatel wrote:

> In D100573#2692336 <https://reviews.llvm.org/D100573#2692336>, @nikic wrote:
>
>> That does also suggest a possible alternative approach here, which is to do the same thing we do for phi nodes and do the recursive queries with MaxDepth-1.
>
> Right - I drafted something with that approach, so I can clean it up and post it if that's preferable.
>
> That's a more limiting approach IIUC because that would cut off all recursive analysis for the computeKnownBits calls within computeKnownBitsFromAssume(). The proposal here still allows recursion through other methods like computeKnownBitsFromOperator(), but only clips before we descend via another round of computeKnownBitsFromAssume().

I think both variants would perform similarly in practice, because these recursive known bits calls are mostly just unnecessary over-generalization. In practice, we expect assume patterns to be things like `(a & 15) == 0` to say that a value is aligned, not `(a & k1) == k2`, where we happen to be able to infer something based on known bits in k1 and k2.

As I don't expect either variant to have much practical impact, I'd be okay either way, but prefer the option you implemented in this patch, because it also gets rid of the assume exclusion mechanism. That's a nice cleanup.


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

https://reviews.llvm.org/D100573