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

Thu Jul 16 05:00:32 PDT 2020

lebedev.ri added subscribers: lattner, spatel.
lebedev.ri added a comment.

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

> Any more on this?

Still looks good to me, but i'd like to see some (positive?) feedback in the actual RFC llvm-dev thread from the usual suspects (@lattner @spatel @nikic)

================
Comment at: llvm/docs/LangRef.rst:14459
 '``llvm.umul.fix.*``' Intrinsics
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

----------------
arsenm wrote:
> Unrelated changes
Please just commit these changes to already-existing lines directly.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:787

+SDValue DAGTypeLegalizer::PromoteIntRes_SHLSAT(SDNode *N) {
+  // Expand it to
----------------
ebevhan wrote:
> arsenm wrote:
> > This code is exactly the same as for ADD/SUB sat and should be shared
> The code is not exactly the same, but some of it is. I could try factoring it in. I find that code that does a lot is a bit harder to read, though.
While i'm usually against duplication, i'm not not very sure that in this case it is better to deduplicate.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83216