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

[Matt Arsenault via llvm-commits]

arsenm 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.
+
----------------
hfinkel wrote:
> 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.
> 
The converse is we already don't 'correctly' lower the existing intrinsics which are assumed to write errno because errno does not exist on the platform.

I'm still generally confused about the inconsistency of errno handling. Why don't we have a separate set of math intrinsics for respecting errno, and not? Lowering the non-errno version with a library call would be an incorrect lowering for these. Alternatively, why doesn't the possibility of of writing errno always be a libcall, while the intrinsics are fine for -fno-math-errno? Currently -fno-math-errno adds readnone to the call site of the library call, and allows selecting to the corresponding DAG node. The inconsistency in behavior between the DAG nodes and intrinsics has always confused me. A readnone call to the library function will select to the corresponding chainless node, which could still be lowered to a call to an errno writing function. In the case of sqrt, this is further confused because < 0 inputs are no longer undefined. I would expect the intrinsics would be the for using a native instruction which ignores errno.

The current set of math intrinsics, including those that say handle errors the same way, are already IntrNoMem (e.g. llvm.exp) and say nothing about undefined behavior. The sqrt intrinsic has undefined behavior for < 0, but we are able to fold an isnan() check before it out in the DAG. I'm not sure what an underflow/overflow test for ldexp would look like, but it would be more complicated than the simple compare and select for sqrt.


http://reviews.llvm.org/D14327