<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><br class=""></div><div class="">Hi,</div><div class="">Those were all very compelling arguments indeed, but I still think it's something we could fix. While other compilers such as gcc are not necessarily an authority on the issue, I think we should avoid inconsistencies.</div><div class=""><br class=""></div><div class="">For example, the fact that sqrt(-2) is undefined, but log(-2) seems to nicely turn into NaN eventually. That feels rather broken to me.</div><div class=""><br class=""></div><div class="">Or the fact that when building with -ffast-math or -Ofast, the behaviour of the code is changes completely (like in the xalanc benchmark, where it jumps into an endless loop because of it and eventually segfaults due to a stack overflow).</div><div class="">The point being that this will introduce a lot of weird side effects in peoples binaries, and I'd rather fix it now in one place in the compiler than have everybody fix it in their (or in 3rd party) code, which most likely is not going to happen and people will happily blame it on the compiler or the way optimisation passes are implemented.</div><div class=""><br class=""></div><div class="">I can try to change it into a more consistent fix, by either:</div><div class="">* removing the transform completely, as suggested by Eli, and leave the rest to the optimiser</div><div class="">* changing the undef to NaN (not very sure what the implications of that would be of even if it is possible, but it seems to be a sensible solution)</div><div class="">* trying to extend the current fix/workaround to cover the cases when the argument is not a constant, but this seems to be a little more complicated</div><div class=""><br class=""></div><div class="">I'll pick one later today, but I am open to suggestions :)</div><div class=""><br class=""></div><div class=""><div class="">@Eli: Good catch! I've noticed this as well, on the other hand from my observations this would only happen if the value of the argument is known (for example if it is specifically assigned in the same scope). It seems to work fine if the argument is calculated somewhere else and passed on, which is probably the typical usecase.</div><div class="">I could remove that transform completely, that was actually my first workaround to the issue.</div><div class="">Thanks!</div></div><br class=""><div><blockquote type="cite" class=""><div class="">On 07 Nov 2016, at 08:51, Michael Kuperstein via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">This has been raised several times (xalanc failing with -ffast-math isn't new), and I believe the last time around we've explicitly decided to keep this as undef.<div class=""><br class=""></div><div class="">A few relevant threads:<br class=""><div class=""><br class=""></div><div class=""><a href="http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20140609/220450.html" class="">http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20140609/220450.html</a><br class=""></div></div><div class=""><a href="http://lists.llvm.org/pipermail/llvm-dev/2014-September/077137.html" class="">http://lists.llvm.org/pipermail/llvm-dev/2014-September/077137.html</a><br class=""></div><div class=""><a href="http://llvm.org/bugs/show_bug.cgi?id=21048" class="">http://llvm.org/bugs/show_bug.cgi?id=21048</a><br class=""></div><div class=""><br class=""></div><div class="">Thanks,</div><div class=""> Michael</div></div><div class="gmail_extra"><br class=""><div class="gmail_quote">On 6 November 2016 at 23:31, Vedant Kumar via llvm-commits <span dir="ltr" class=""><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The optimizer doesn't handle this right now, although it probably could since<br class="">
-instsimplify runs pretty late in the pipeline.<br class="">
<br class="">
Do you think extending the LibCallSimplifier is the right approach?<br class="">
<br class="">
best,<br class="">
vedant<br class="">
<br class="">
P.S: w.r.t the problem that motivated this patch, I think a better solution<br class="">
would be to go through the test programs and change their uses of sqrt().<br class="">
<div class="HOEnZb"><div class="h5"><br class="">
<br class="">
> On Nov 6, 2016, at 9:31 AM, Friedman, Eli via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class="">
><br class="">
> On 11/4/2016 11:15 AM, Octav Zlatior via llvm-commits wrote:<br class="">
>><br class="">
>> Hi,<br class="">
>><br class="">
>> 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.<br class="">
>><br class="">
>> 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).<br class="">
>><br class="">
>> 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.<br class="">
>><br class="">
>> 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.<br class="">
>><br class="">
><br class="">
> 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.<br class="">
><br class="">
> Maybe we should just kill off this transform completely? The optimizer knows how to handle a regular sqrt() call anyway.<br class="">
><br class="">
> -Eli<br class="">
><br class="">
> --<br class="">
> Employee of Qualcomm Innovation Center, Inc.<br class="">
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project<br class="">
><br class="">
> ______________________________<wbr class="">_________________<br class="">
> llvm-commits mailing list<br class="">
> <a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/<wbr class="">mailman/listinfo/llvm-commits</a><br class="">
<br class="">
______________________________<wbr class="">_________________<br class="">
llvm-commits mailing list<br class="">
<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/<wbr class="">mailman/listinfo/llvm-commits</a><br class="">
</div></div></blockquote></div><br class=""></div>
_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits<br class=""></div></blockquote></div><br class=""><div class="">
<div style="color: rgb(0, 0, 0); letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">— </div><div class="">Octav Zlatior</div><div style="color: rgb(0, 0, 0); letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div style="color: rgb(0, 0, 0); letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Theobroma Systems Design und Consulting GmbH<br class="">Seestadtstrasse 27, 1220 Wien, Austria<br class="">Phone: +43 1 2369893-407<br class=""><a href="http://www.theobroma-systems.com" class="">http://www.theobroma-systems.com</a></div><div class=""><br class=""></div></div><br class="Apple-interchange-newline"></div><br class="Apple-interchange-newline"></div><br class="Apple-interchange-newline"><br class="Apple-interchange-newline">
</div>
<br class=""></body></html>