[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:14035
+The '``llvm.experimental.constrained.fptrunc``' intrinsic returns the result of
+a truncating of a floating point operand into a smaller floating point result.
+
----------------
craig.topper wrote:
> This reads funny. I think it should maybe be "result of truncating a floating point"
How about if I just copy the text used by the normal fptrunc instruction?


================
Comment at: docs/LangRef.rst:14069
+The '``llvm.experimental.constrained.fpext``' intrinsic returns the result of
+an enlarging of a floating point operand.
+
----------------
craig.topper wrote:
> This also reads funny
Same as fptrunc. I could just copy the text from the fpext instruction?


================
Comment at: include/llvm/CodeGen/ISDOpcodes.h:528
 
+    STRICT_FP_TO_SINT,
+    STRICT_FP_TO_UINT,
----------------
craig.topper wrote:
> These need comments. Does STRICT_FP_ROUND have the TRUNC argument that FP_ROUND has? 
I could rearrange and lump them in with the non-strict versions of each. Then I'd just need one extra line restating that the STRICT_ versions prevent optimizations.

Yes, STRICT_FP_ROUND does have the TRUNC argument that FP_ROUND has, but it is currently always zero. Fixing this require rerouting this one strict node to go through the same codepath as the non-strict node. That would make it different from all the other constrained nodes. Should I go ahead and make that change? I _think_ it is safe if the TRUNC argument really does work like it is documented.

I also just noticed that I need to put back STRICT_FP_TO_UINT in at least one place. It should be everywhere STRICT_FP_TO_SINT is handled _except_ in the default lowering.


https://reviews.llvm.org/D43515