[PATCH] D112298: [InstCombine] Generalize sadd.sat combine to compute sign bits.

[Dave Green via Phabricator via llvm-commits]

dmgreen marked 8 inline comments as done.
dmgreen added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:1273-1274
   // the operands of the add are 64 bits wide, we need at least 33 sign bits.
-  unsigned NeededSignBits = CI1->getBitWidth() - NewWidth + 1;
-  if (IC.ComputeNumSignBits(A, 0, &I) < NeededSignBits ||
-      IC.ComputeNumSignBits(B, 0, &I) < NeededSignBits)
+  if (IC.ComputeMinSignedBits(A, 0, &I) > NewWidth ||
+      IC.ComputeMinSignedBits(B, 0, &I) > NewWidth)
     return nullptr;
----------------
lebedev.ri wrote:
> I think this is a NFC change, in which case can you precommit it first?
Thanks. Part1: rG61225c081858


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:2318-2319
+  // is usually achieved via a sext from a smaller type.
+  if (ComputeMinSignedBits(AddSub->getOperand(0), 0, AddSub) > NewBitWidth ||
+      ComputeMinSignedBits(AddSub->getOperand(1), 0, AddSub) > NewBitWidth)
+    return nullptr;
----------------
lebedev.ri wrote:
> These aren't using InstCombine's wrapper?
This function is in InstCombinerImpl, so this should go through the wrappers. The other part in processUGT_ADDCST_ADD needs to call them via IC that is passed to the function.


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

https://reviews.llvm.org/D112298