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

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:5428
   if (!Scale) {
-    if (VT.isVector() && !isOperationLegalOrCustom(ISD::MUL, VT))
-      return SDValue();
-    return DAG.getNode(ISD::MUL, dl, VT, LHS, RHS);
+    // [us]mul.fix(a, b, 0) -> mul(a, b)
+    if (!Saturating && isOperationLegalOrCustom(ISD::MUL, VT)) {
----------------
RKSimon wrote:
> I think you need something like this here (please double check my logic):
> ```
> if (VT.isVector() && !isOperationLegalOrCustom(ISD::SMULO, VT) &&
>     !(!Saturating && isOperationLegalOrCustom(ISD::MUL, VT)))
>   return SDValue(); // unroll
> ```
> And that will let you avoid the return SDValue() below by always defaulting to a scalar ISD::MUL/ISD::SMULO that legalization can handle.
I've committed rL353546 which /should/ mean that the scale==0 case is now safe to drop through.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55720