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

[Nikita Popov via Phabricator via llvm-commits]

nikic accepted this revision.
nikic added a comment.
This revision is now accepted and ready to land.

> The suggestion was to make the exclusion logic stronger to avoid blowing up, but I'm not seeing how to do that. Note that we reduced the complexity of the exclusion mechanism in D16204 <https://reviews.llvm.org/D16204> because it was too costly.

What I had in mind was to exclude not a specific assume, but all assumes on a given value (the value passed to assumptionsFor). However, this change by itself did not fix the issue (only in conjunction with a limit on number of assumes visited in the loop).

I like your idea here though. Recursing over assumes does seem to be of rather questionable value. For the affected example, if we use less silly instruction ordering, GVN would canonicalize the variables (https://llvm.godbolt.org/z/1cn93nEh1) after which the recursion is no longer necessary.

So this LGTM. Maybe wait a bit in case someone has an alternative suggestion.


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

https://reviews.llvm.org/D100573