[PATCH] D55720: [Intrinsic] Signed Fixed Point Saturation Multiplication Intrinsic

Bevin Hansson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 7 00:34:42 PST 2019


ebevhan added inline comments.


================
Comment at: llvm/lib/CodeGen/TargetLoweringBase.cpp:627
     setOperationAction(ISD::SMULFIX, VT, Expand);
+    setOperationAction(ISD::SMULFIXSAT, VT, Expand);
     setOperationAction(ISD::UMULFIX, VT, Expand);
----------------
bjope wrote:
> I'm not sure how to do this when overriding for a specific target. In our case we want it to be legal, but only when the scale is 15 (and VT is i16 or i24) or the scale is 31 (and VT is i32 or i40). Is there some easy solution for that? Setting it to legal/custom for any scale might be seen as an indication for optimizers that it is OK to introduce these operations for any scale.
> 
> This is however a general comment, also for the already pushed non-saturating versions. So it isn't anything that you need to deal with in this patch. But we might need a better solution in the long term.
That's what the `isSupportedFixedPointOperation` hook in TargetLowering is for.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55720





More information about the llvm-commits mailing list