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

[Nikita Popov via Phabricator via llvm-commits]

nikic marked an inline comment as done.
nikic added inline 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,
----------------
lebedev.ri wrote:
> 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 {}`.
Agree on the suffix being unnecessary. Regarding anonymous namespaces, https://llvm.org/docs/CodingStandards.html#anonymous-namespaces says:

> Because of this, we have a simple guideline: make anonymous namespaces as small as possible, and only use them for class declarations.

If I'm understanding correctly, this means that anon namespaces should be used for classes and static for functions.


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

https://reviews.llvm.org/D60522