[PATCH] D151816: [ValueTracking] Directly use KnownBits shift functions

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:994
+      Known.isNonZero() ||
+      (Known.getMaxValue().ult(Known.getBitWidth()) &&
+       isKnownNonZero(I->getOperand(1), DemandedElts, Depth + 1, Q));
----------------
nikic wrote:
> goldstein.w.n wrote:
> > nikic wrote:
> > > goldstein.w.n wrote:
> > > > what is the `Known.getMaxValue().ult(Known.getBitWidth())` is doing here?
> > > Nothing semantically meaningful. It only avoids calling isKnownNonZero() in substantially more cases than the old code.
> > How does Known.getMaxValue() u< Known.getBitWidth() prevent Known from being 0?
> It doesn't. Note the `&&` rather than `||`. It's a heuristic on when it is profitable to call isKnownNonZero() that matches what the old code did. It doesn't make a whole lot of sense, but I'd like to retain the behavior for the purposes of this patch.
Ah, makes sense I had misread as (Known.isNonZero() || MaxVal < GetBitwidth) || isKnownNonZero.


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

https://reviews.llvm.org/D151816