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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:2592
+  SDValue SRLAmnt = DAG.getConstant(Scale, dl, ShiftTy);
+  SDValue SHLAmnt = DAG.getConstant(NVTSize - Scale, dl, ShiftTy);
+
----------------
if Scale is grater than NVTSize then this number is negative.


================
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);
----------------
Why is ResultLL being shifted right twice? I don't think I understand that.


================
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));
----------------
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.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54719