[PATCH] D60522: [KnownBits] Add computeForAddCarry()

[Roman Lebedev via Phabricator via llvm-commits]

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

LG modulo nits, if @RKSimon has no further comments.



================
Comment at: llvm/lib/Support/KnownBits.cpp:18
 
-KnownBits KnownBits::computeForAddSub(bool Add, bool NSW,
-                                      const KnownBits &LHS, KnownBits RHS) {
-  // Carry in a 1 for a subtract, rather than 0.
-  bool CarryIn = false;
-  if (!Add) {
-    // Sum = LHS + ~RHS + 1
-    std::swap(RHS.Zero, RHS.One);
-    CarryIn = true;
-  }
+static KnownBits computeForAddCarryInternal(
+    const KnownBits &LHS, const KnownBits &RHS,
----------------
The signature is already different from `computeForAddCarry()` (2 `bool`s vs 1 `KnownBits`),
i don't see the need for `Internal` suffix.
Also, i believe anonymous namespaces are preferred, so drop `static`, and wrap into `namespace {}`.


================
Comment at: llvm/lib/Support/KnownBits.cpp:54
+
+KnownBits KnownBits::computeForAddSub(bool Add, bool NSW,
+                                      const KnownBits &LHS, KnownBits RHS) {
----------------
All these `bool` params are potential bugs, and really affect readability.
But i guess that would be too intrusive for this header.


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

https://reviews.llvm.org/D60522