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

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

LGTM! (with a nit in the test case)

I also think we should consider to add more exhaustive tests for computeForAddSub, now when we got the unittest support from this patch.



================
Comment at: llvm/unittests/Support/KnownBitsTest.cpp:1
+//===- llvm/unittest/Support/KnownBits.cpp - KnownBits tests --------------===//
+//
----------------
nit: KnownBitsTest.cpp


================
Comment at: llvm/unittests/Support/KnownBitsTest.cpp:48
+}
+
+TEST(KnownBitsTest, AddCarryExhaustive) {
----------------
The patch slightly touches the computeForAddSub implementation. So even if it can be seen as unrelated, maybe we should add a exhaustive test for computeForAddSub as well. Shouldn't be too hard given this framework, or what do you think?


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

https://reviews.llvm.org/D60522