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

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Sun Nov 6 23:31:08 PST 2016


The optimizer doesn't handle this right now, although it probably could since
-instsimplify runs pretty late in the pipeline. 

Do you think extending the LibCallSimplifier is the right approach?

best,
vedant

P.S: w.r.t the problem that motivated this patch, I think a better solution
would be to go through the test programs and change their uses of sqrt().


> On Nov 6, 2016, at 9:31 AM, Friedman, Eli via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> 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
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list