[PATCH] D12799: [ValueTracking] Teach isKnownNonZero a new trick

[James Molloy via llvm-commits]

jmolloy marked 4 inline comments as done.

================
Comment at: lib/Analysis/ValueTracking.cpp:1837
@@ +1836,3 @@
+        APInt LoBits = APInt::getLowBitsSet(BitWidth, Shift->getLimitedValue());
+        if ((KnownZero & LoBits).eq(LoBits))
+          return KnownOne.getBoolValue() || isKnownNonZero(X, DL, Depth, Q);
----------------
sanjoy wrote:
> sanjoy wrote:
> > reames wrote:
> > > This really seems like two distinct cases and the code would be clearer if organized as such.
> > > 
> > > case 1 - There's a known one bit somewhere in the portion not shifted out.
> > > 
> > > case 2 - All bits shifted out are known-zero and X is known non-zero.  
> > > 
> > > I think what you have is correct, I just found the flow of the code confusing on first read.  
> > You could also check for the `exact` flag here.
> How about `if (KnownZero.countTrailingZeros() >= Shift->getLimitedValue())`?
I think the exact flag is tested for just above.

================
Comment at: lib/Analysis/ValueTracking.cpp:1838
@@ +1837,3 @@
+        if ((KnownZero & LoBits).eq(LoBits))
+          return KnownOne.getBoolValue() || isKnownNonZero(X, DL, Depth, Q);
+      }
----------------
sanjoy wrote:
> I'd expect this first case to be handled by the fall through at the end.  Do you have a case where it doesn't?
Nope - this was purely a compile-speed micro optimization - we've already computed known bits, so it seemed a waste to compute them again.


Repository:
  rL LLVM

http://reviews.llvm.org/D12799