fast-math: fix a bug related to square roots of negatives

[Friedman, Eli via llvm-commits]

On 11/4/2016 11:15 AM, Octav Zlatior via llvm-commits wrote:
>
> Hi,
>
> I have noticed the following behaviour while running the SPEC 
> benchmark suite (built with llvm, of course). One of the tests was 
> failing (Xalanc) with a segfault (because of stack overflow) because 
> the terminating condition for a recursion involved NaN.
>
> I've checked a little further and I have noticed that, with fast-math, 
> the square roots of negative numbers will sometimes be replaced with 
> "return undef". But only sometimes, and only the square roots (for 
> example, log(x) does not manifest this behaviour).
>
> It turns out a rather common way of getting the value of NaN in code 
> is to assign it the return value of sqrt(<negative number>). Because 
> of this bug, any such assignments will basically result undefined values.
>
> These two patches should provide a fix for this. The first is a test 
> to illustrate the situation (the negative functions will be replaced 
> with return undef in the IR), while the other is a fix for this bug.
>

Your patch specifically says that "sqrt(-1)" should be lowered to NaN, 
but "float x = -1.f; sqrt(x)" should be lowered to the intrinsic, which 
has undefined behavior according to LangRef.  This seems dubious; 
changing the semantics of sqrt() based on the syntactic form of the 
source is likely to cause problems.  For example, writing a trivial 
wrapper for sqrt (like the one in libcxx for sqrtf) breaks your check.

Maybe we should just kill off this transform completely?  The optimizer 
knows how to handle a regular sqrt() call anyway.

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project