[PATCH] D54719: [Intrinsic] Signed Fixed Point Multiplication Intrinsic

[Leonard Chan via Phabricator via llvm-commits]

leonardchan added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:2607
+    // third part in the result (ResultHL).
+    ResultLL = DAG.getNode(ISD::SRL, dl, NVT, ResultLL, SRLAmnt);
+    Lo = DAG.getNode(ISD::SRL, dl, NVT, ResultLL, SRLAmnt);
----------------
craig.topper wrote:
> Why is ResultLL being shifted right twice? I don't think I understand that.
My bad. It shouldn't have shifted twice.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:2624
+    Hi = DAG.getNode(ISD::SRL, dl, NVT, ResultHL, SRLAmnt);
+    Hi = DAG.getNode(ISD::OR, dl, NVT, Hi,
+                     DAG.getNode(ISD::SHL, dl, NVT, ResultHH, SHLAmnt));
----------------
craig.topper wrote:
> If Scale == NVTSize then the result is exactly in ResultHL and ResultLH, but nothing can prevent at least 1 bit of ResultHH from being used here. The shift amount for the ResultHH shift must be between 0 and NVTSize-1 to not be undefined.
Done. Added test case for this also.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54719