[PATCH] D52286: [Intrinsic] Signed Saturation Intirnsic

[Bevin Hansson via Phabricator via llvm-commits]

ebevhan added a comment.

In https://reviews.llvm.org/D52286#1253162, @craig.topper wrote:

> I'm not sure why you would need an intrinsic at all. Why can't you just emit the IR equivalent from the beginning. This looks the equivalent of doing something like "x = std::min(std::max(x, -2048), 2047)" in C++ which would be implemented in IR with 2 compares and 2 selects. This is not an uncommon pattern to find in real code and we spend a lot of effort to make sure its handled well. Adding an intrinsic means you'll miss out on the constant folding, knownbits, etc that we've already put in for min/max IR patterns.


The issue with using pure IR for these operations is that if the optimizer so much as sneezes on the representation of the operation, there's a high risk that you won't properly select a native operation. This is extremely important on targets for which these operations -- fixed-point in particular, but also saturation -- are legal, like our downstream one.

It is pretty unfortunate that it means we can't take advantage of a lot of the work put into min-max patterns, which is one of the reasons why I thought an early lowering-to-IR pass would be interesting to consider on targets for which these operations are not legal, rather than doing the expansion as late as isel.


Repository:
  rL LLVM

https://reviews.llvm.org/D52286