[PATCH] D55625: [Intrinsic] Unsigned Fixed Point Multiplication Intrinsic

[Bevin Hansson via Phabricator via llvm-commits]

ebevhan added a comment.

Maybe a test with scale == bitwidth? Other than that it looks good to me.



================
Comment at: llvm/docs/LangRef.rst:12993
+
+The arguments (%a and %b) and the result may be of integer types of any bit
+width, but they must have the same bit width. ``%a`` and ``%b`` are the two
----------------
It takes vectors as well, though. Seems like the smul.fix docs only mention integers as well.


================
Comment at: llvm/docs/LangRef.rst:13023
+      ; The result in the following could be rounded down to 3.5 or up to 4
+      %res = call i4 @llvm.smul.fix.i4(i4 15, i4 1, i32 1)  ; %res = 7 (or 8) (7.5 x 0.5 = 3.75)
+
----------------
This says smul.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5843
   }
+  case Intrinsic::umul_fix: {
+    SDValue Op1 = getValue(I.getArgOperand(0));
----------------
Is there a reason this doesn't reuse the code above?


================
Comment at: llvm/lib/IR/Verifier.cpp:4561
+        Op3->getZExtValue() < Op1->getType()->getScalarSizeInBits(),
+        "the scale of [us]mul_fix must be less than the width of the operands");
     break;
----------------
This can't be true for umul_fix.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55625