[PATCH] D43515: More math intrinsics for conservative math handling

[Kevin P. Neal via Phabricator via llvm-commits]

kpn added inline comments.


================
Comment at: docs/LangRef.rst:13325
+
+      declare <type>
+      @llvm.experimental.constrained.fptrunc(<type> <op>,
----------------
andrew.w.kaylor wrote:
> You need to do something more here to document the difference between the return type and the argument type. Also in fpext below.
I hope this is what you mean. I've changed it to show that the result is a different type. I've followed the naming scheme used elsewhere in this document.


================
Comment at: lib/CodeGen/StrictFP.cpp:144
+  auto *t = cast<IntegerType>(I->getType());
+  APInt IntMaxAP(DL->getTypeStoreSize(t) * 8, t->getSignBit());
+  APFloat FPMaxAP((double)0);
----------------
andrew.w.kaylor wrote:
> I think you can use "IntDst->getBitWidth()" here. Also, APInt::getSignedMinValue() does this same thing and is a bit more self-documenting.
error: no member named `getBitWidth' in `llvm::Value'.


================
Comment at: lib/IR/Verifier.cpp:4526
+  if (HasRoundingMD) {
+    int RoundingOffset = (HasExceptionMD ? 2 : 1);
+    Assert(isa<MetadataAsValue>(FPI.getArgOperand(NumOperands-RoundingOffset)),
----------------
andrew.w.kaylor wrote:
> How about this?
> 
> int RoundingIdx = (HasExceptionMD ? NumOperands - 2 : NumOperands - 1);
> 
> On the other hand, are we ever going to have intrinsics that have a rounding mode but not exception behavior?
Done.

I don't know if we'll have a rounding mode but not exceptions. I doubt it, but I can't say for certain.


https://reviews.llvm.org/D43515