[PATCH] D14327: Add llvm.ldexp.* intrinsic, associated SDNode and library calls

[Hal Finkel via llvm-commits]

hfinkel added inline comments.

================
Comment at: docs/LangRef.rst:9989
@@ +9988,3 @@
+power. If the first argument is NaN or infinite, the same value is returned.
+The behavior in case of a range error (underflow or overflow) is undefined.
+
----------------
arsenm wrote:
> The returned value on underflow is defined to be zero, and HUGE_VAL, which may be infinity, on overflow. I think saying undefined behavior for the case is too strong. Maybe saying just the state of errno is undefined?
We don't have a way to model errno. We need to "prevent" a situation where we're allowed to reorder a call to ldexp in between, for example, a call to open() and a call to perror(). To get the benefits you want, however, you need to mark the function as readnone. However, it might be implemented using the underlying library call, which might set errno. Unless you make that undefined behavior, then the readnone on the intrinsic is wrong. Both overflow and underflow need to be undefined behavior. I realize that this is unfortunate.


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:465
@@ -464,2 +464,3 @@
       setOperationAction(ISD::FPOWI, VT, Expand);
+      setOperationAction(ISD::FLDEXP, VT, Expand);
       setOperationAction(ISD::FFLOOR, VT, Expand);
----------------
Don't do this. Set it to Expand by default (in TargetLoweringBase::initActions). That's our current best practice for new rarely-legal nodes.


http://reviews.llvm.org/D14327