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

Wed Jul 8 07:49:28 PDT 2020

ebevhan added a comment.

In D83216#2138974 <https://reviews.llvm.org/D83216#2138974>, @lebedev.ri wrote:

> Patch as-is looks good but i'm not sure what's the RFC status here.
>  If these new intrinsics were already previously proposed as part of some RFC that got accepted,
>  can you state that in the patch's description? (with link to the thread)

I added the links to the threads I mentioned earlier.

Looking back at the full discussion, it doesn't really seem like any real consensus regarding **how** to implement the types was reached, but the prevailing view was that an altogether new IR type was the best approach. I don't think either I or Leonard thought that was the right (or fastest, at least) way to go, though.

The final listing of intrinsics was in http://lists.llvm.org/pipermail/llvm-dev/2018-September/126311.html but the design has diverged a bit from that since then.

In D83216#2139104 <https://reviews.llvm.org/D83216#2139104>, @lebedev.ri wrote:

> In D83216#2139098 <https://reviews.llvm.org/D83216#2139098>, @ebevhan wrote:
>
> > Fixed review comment and updated summary.
>
>
> (note that updating commit msg does not automatically update description in phab differential)

I noticed! I was looking for an arc option to do that but couldn't seem to find one.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83216