[PATCH] D83216: [Intrinsic] Add sshl.sat/ushl.sat, saturated shift intrinsics.

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

In D83216#2196048 <https://reviews.llvm.org/D83216#2196048>, @ebevhan wrote:

> In D83216#2196028 <https://reviews.llvm.org/D83216#2196028>, @lebedev.ri wrote:
>
>> LGTM, but i would have preferred to see more feedback on the RFC thread.
>> That being said, unless you are planning on forming calls to these intrinsics in middle-end transform passes
>> (i.e. only planning on using them in clang codegen), i think this is okay to proceed.
>
> At least for now, I'm not planning to emit them in middleend. I wanted to get most of the basic support in Clang and LLVM before looking at optimization.
>
> However, based on the discussions in D82663 <https://reviews.llvm.org/D82663>, I am planning on sending an RFC for moving the fixed-point semantics and value class to LLVM, and adding a Builder class similar to the MatrixBuilder.

I think middle-end rewrites such as constant folding, or rewriting (sshl.sat (sshl.sat X, Y), Z) -> (sshl.sat X, Y+Z) is OK.
As Roman said, we should probably think twice before we start rewrite arbitrary things into using these intrinsics (such as rewriting a saturated multiply into a saturated shift).

Still, we probably want to have a canonical form for things like "saturated multiplication by 2" in the future. I think that a lot of these saturated left shifts can be done using a saturated multiplication. Similarly saturated right shifts can often be described using saturated division. At least when the shift count is known. For unknown shift counts these intrinsics will make it much easier for a backend target that has such shifts in the instruction set to do the right thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83216