[PATCH] D60460: [SelectionDAG] Use KnownBits::computeForAddSub in SelectionDAG::computeKnownBits

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: llvm/unittests/CodeGen/AArch64SelectionDAGTest.cpp:160
 
+// Piggy-backing on the AArch64 tests to verify SelectionDAG::computeKnownBits.
+TEST_F(AArch64SelectionDAGTest, ComputeNumSignBits_ADD) {
----------------
lebedev.ri wrote:
> If a diff is based on some other patch, it is best not to include that other patch into this diff (be more specific for which commits you want to generate the diff in git), but instead mark that other patch as parent.
Not sure if I get that. You asked for test cases, and these are the test cases I added (they would fail without the improvements in computeKnownBits for ADD and SUB).

Although, I see now that I made a typo in the test names. They should be called ComputeKnownBits_ADD and ComputeKnownBits_SUB.

However, if I rebase this upon https://reviews.llvm.org/D60522 this patch will use the helpers in KnownBits (which also includes more exhaustive unittests). So I guess these tests will become less important after that rebase.


================
Comment at: llvm/unittests/CodeGen/AArch64SelectionDAGTest.cpp:161
+// Piggy-backing on the AArch64 tests to verify SelectionDAG::computeKnownBits.
+TEST_F(AArch64SelectionDAGTest, ComputeNumSignBits_ADD) {
+  if (!TM)
----------------
typo: ComputeKnownBits_ADD


================
Comment at: llvm/unittests/CodeGen/AArch64SelectionDAGTest.cpp:182
+// Piggy-backing on the AArch64 tests to verify SelectionDAG::computeKnownBits.
+TEST_F(AArch64SelectionDAGTest, ComputeNumSignBits_SUB) {
+  if (!TM)
----------------
typo: ComputeKnownBits_SUB


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60460