[PATCH] D68651: [InstCombine] Signed saturation patterns

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

Generally looks good to me, I'm only wondering whether the `trunc` is the right place to start the match. Starting from the min/max we could match a larger set of patterns, in particular those where the result of the saturation is still extended to a larger type -- for example doing a 16-bit saturating add but continuing with a 32-bit result.



================
Comment at: llvm/include/llvm/IR/PatternMatch.h:663
+/// Match a specified integer value or vector of all elements of that
+// value.
+struct specific_apintval {
----------------
nit: `///`


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:729
+  if (A->getType() != Ty || B->getType() != Ty)
+    return nullptr;
+
----------------
Rather than exact type equality, we could require that the original type is <= the trunc type and sext to the trunc type. This would allow also matching a saturating add between 16-bit and 8-bit number, for example. Not sure how practically relevant that would be though.


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

https://reviews.llvm.org/D68651