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

Bevin Hansson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 7 01:48:04 PDT 2020


ebevhan marked an inline comment as done.
ebevhan added a comment.

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

> Was there an RFC for this?


No explicit RFC for these particular intrinsics, but they have been mentioned in the larger scope of fixed-point support:

- http://lists.llvm.org/pipermail/llvm-dev/2018-August/125433.html
- http://lists.llvm.org/pipermail/cfe-dev/2018-May/058019.html

I notice now that Leonard's mail does not mention saturated shifts, but my older one does.

> While i agree it likely makes sense to have these for consistency,
>  i'm not sure why they are *needed* for implementing the Embedded-C fixed point support in Clang.

Yes, "needed" might be a stronger wording than necessary. I originally wrote "useful" but was concerned it wasn't strong enough.
Of course, they aren't needed per se, but it becomes more of a hassle to select instructions for the operations if there are no intrinsics.



================
Comment at: llvm/lib/IR/Verifier.cpp:4948-4951
+    Assert(Op1->getType()->isIntegerTy(),
+           "first operand of [us]shl_sat must be an int type");
+    Assert(Op2->getType()->isIntegerTy(),
+           "second operand of [us]shl_sat must be an int type");
----------------
lebedev.ri wrote:
> I don't think it makes sense to limit these to scalars.
The add.sat and sub.sat intrinsics were given vector operands because they were useful for some of x86's vector instructions. I couldn't see any such operations for shifts, but I can add the vector type support for consistency.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83216





More information about the llvm-commits mailing list