[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:20-21
+    const KnownBits &LHS, const KnownBits &RHS,
+    bool CarryZero, bool CarryOne) {
+  assert(!(CarryZero && CarryOne) &&
+         "Carry can't be zero and one at the same time");
----------------
lebedev.ri wrote:
> How about simply making it `KnownBits(/*KnownBits=*/1)` ?
It's a possibility, but I'm not convinced it's better here. The booleans directly enforce the contract that this is just one bit (while for KnownBits we could only assert that), they're cheaper to construct and use than two APInts, and the API is simpler to use, because we don't have to construct KnownBits (which is somewhat roundabout due to lack of a direct ctor).

I think the only place that would really benefit from passing carry as KnownBits is the testing code, while for other usages having this as booleans is more convenient and efficient.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60522