[PATCH] D32686: [InstCombine][KnownBits] Use KnownBits better to detect nsw adds

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:853
+/// Otherwise return false.
+static bool checkRippleForAdd(const KnownBits &RHSKnown,
+                              const KnownBits &LHSKnown) {
----------------
The these parameter names are swapped relative to the caller. Prefer LHS followed by RHS.


================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:866
+  // there is no overflow to the sign bit.
+  if(RHSKnown.isNonNegative() || LHSKnown.isNonNegative()) {
+    APInt MaxRHS = ~RHSKnown.Zero;
----------------
craig.topper wrote:
> Space between 'if' and 'parenthese'
Also do LHS check first then RHS. Basically as much as possible do everything for LHS before RHS.


================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:871
+    MaxLHS.clearSignBit();
+    APInt Result = MaxLHS + MaxRHS;
+    return Result.isSignBitClear();
----------------
craig.topper wrote:
> Wrap both MaxLHS and MaxRHS in std::move so we can reuse one of their allocations to hold the addition result when dealing with larger than 64-bit numbers.
Or better yet, maybe just use MaxLHS += MaxRHS.


https://reviews.llvm.org/D32686