[PATCH] D142271: [ValueTracking] Add KnownBits patterns `xor(x, x - 1)` and `and(x, -x)` for knowing upper bits to be zero

[Nikita Popov via Phabricator via llvm-commits]

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

LGTM



================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:1096
+        else
+          Known = Known2.blsi();
+      }
----------------
goldstein.w.n wrote:
> nikic wrote:
> > break here? Don't think we want to fall through.
> > break here? Don't think we want to fall through.
> 
> Fallthrough here has no issue. While I think the `blsi` case is mutually exclusive with the other transform, I generally think breaking early in this type of code (where you have back-to-back cases) can make things unclear as the assumption is generally we will try them all.
At least with this current code structure, falling through is pretty confusing.  `Known` was previously the known bits of the second operand (that is either X or -X) and now it will be the known bits of `X & -X`. I guess this is fine in practice because `&` is idempotent, so we end up computing something like `X & X & -X`.

The new code structure in D142427 avoids this problem, but for this patch I would not do the fall through.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142271