[PATCH] D18777: [ValueTracking] An improvement to IR ValueTracking on Non-negative Integers

[Sanjoy Das via llvm-commits]

sanjoy requested changes to this revision.
sanjoy added a comment.
This revision now requires changes to proceed.

Mostly looks okay, I just have a few minor nits.


================
Comment at: lib/Analysis/ValueTracking.cpp:1248
@@ +1247,3 @@
+          if (OverflowOp && OverflowOp->hasNoSignedWrap()) {
+            // If Initial value of recurrence is nonnegative, and we are adding 
+            // a nonnegative number with nsw, the result can only be nonnegative
----------------
s/Initial/initial/

================
Comment at: lib/Analysis/ValueTracking.cpp:1255
@@ +1254,3 @@
+            //
+            // (add non-negative, non-negative) --> non-negative
+            // (add negative, negative) --> negative
----------------
Nit: I'd use `setBit` instead of `|=` with a new `APInt`.  For `BitWidth` larger than 64, what you have currently will allocate unnecessarily, and I think `setBit` will look just as obvious (if not more).

================
Comment at: lib/Analysis/ValueTracking.cpp:1275
@@ +1274,3 @@
+            else if (Opcode == Instruction::Mul && KnownZero2.isNegative() && 
+                KnownZero3.isNegative())
+              KnownZero |= APInt::getSignBit(BitWidth);
----------------
Indent looks off -- please use clang-format.


https://reviews.llvm.org/D18777