[PATCH] D52286: [Intrinsic] Signed Saturation Intirnsic

[Bevin Hansson via Phabricator via llvm-commits]

ebevhan added a comment.

In https://reviews.llvm.org/D52286#1258497, @bjope wrote:

> If the use of a special intrinsic for saturation is rare (few targets that supports it natively), then perhaps we should put this particular patch on hold for now?
>
> I guess the target that @bevinh (and I) is working on can override the codegen in clang to select our target specific intrinsic just like we do today. Or is it hard to do target specific overrides in clang codegen?


One of the reasons for moving to an intrinsic or new type based solution was to avoid having to implement a custom codegen hook in Clang for the operations for targets that wanted to do it differently. If we wanted to do it that way we would probably have to maintain a patch.

> The generic support for fixed point types could, at least initially, use the min/max clamping. It would let us move forward on the more interesting parts of fixed point support (such as saturated add, multiplication etc), instead of getting stuck in heavy discussions regarding something that IMHO has lower priority. Then we can revisit this patch later. Perhaps it will be easier to demonstrate the need for an ssaturate intrinsic when we come further and clang is generating fixed point conversions.

You could be right. I was aiming for a solution that was generic enough to work for anyone's use case (and also worked for us out of the box), but maybe it's simply not important enough to warrant it.


Repository:
  rL LLVM

https://reviews.llvm.org/D52286