<div dir="ltr">On Thu, Sep 12, 2013 at 2:44 PM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="im">----- Original Message -----<br>
><br>
> On Thu, Sep 12, 2013 at 2:03 PM, Hal Finkel < <a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a> ><br>
> wrote:<br>
><br>
><br>
><br>
><br>
> Hello,<br>
><br>
> Please review the attached patch which restores the libm sqrt* -><br>
> @llvm.sqrt* mapping, but only in fast-math mode (specifically, when<br>
> the UnsafeFPMath or NoNaNsFPMath CodeGen options are enabled). The<br>
> @llvm.sqrt* intrinsics have slightly different semantics from the<br>
> libm call, specifically, they are undefined when given a non-zero<br>
> negative number (the libm calls will always return NaN for any<br>
> negative number).<br>
><br>
> This mapping was removed in r100613, and replaced with a TODO, but at<br>
> that time the fast-math flags were not yet implemented. Now that we<br>
> have these, restoring this mapping is important because it will<br>
> enable autovectorization of sqrt calls in loops (at least in<br>
> fast-math mode).<br>
><br>
><br>
><br>
><br>
> This is dangerous, if LangRef is actually correct. People don't<br>
> associate -ffast-math with "my program will crash at random". :) Of<br>
> course, LangRef is probably overstating the issue.<br>
<br>
</div>I agree, and the LangRef does indeed say "undefined behavior", but I assume that should really mean, "returns an undefined value." Do you agree?<br></blockquote><div><br></div><div>Well, if we map llvm.sqrt to sqrt and sqrt sets errno, we really do mean "undefined behavior"... or at least something more that "returns an undefined value".</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="im">
><br>
><br>
> That said, there's actually a general issue here: if we map the LLVM<br>
> intrinsics to libc functions, and the libc functions set errno, we<br>
> could break code that depends on errno for non-math calls (e.g.<br>
> fopen().)<br>
<br>
</div>Perhaps, but I'm not changing that here. For one thing, if the mapping does, in effect, sqrt -> llvm.sqrt -> sqrt, and only when -fmath-errno=0. Are you worried about cases where the libm functions actually do set errno (even though we have -fmath-errno=0)?<br>
<br></blockquote><div><br></div></div></div><div class="gmail_extra">I'm not sure our implementation of -fno-math-errno is safe: according to the gcc manual, it isn't equivalent to marking the math functions with attribute((const)).  (For example, the gcc manual's definition allows transforming a call to sqrt() into the SSE sqrt instruction, but it doesn't allow hoisting a call to sqrt out of arbitrary loops on a machine where the sqrt() call could set errno.)</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">-Eli</div></div>