<div dir="ltr">On Fri, Sep 6, 2013 at 3:32 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=""><div class="h5">----- Original Message -----<br>
><br>
> On Fri, Sep 6, 2013 at 8:27 AM, Hal Finkel < <a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a> > wrote:<br>
><br>
><br>
><br>
><br>
> Ben, Chad, et al.,<br>
><br>
> The attached patch cleans up the LIBBUILTIN libm function<br>
> definitions, and improves the way in which -ftrapping-math is<br>
> handled in a complementary way. Specifically, this patch:<br>
><br>
> 1. The libm LIBBUILTIN definitions are synchronized with the<br>
> __builtin_* definitions for libm functions. We currently have<br>
> __builtin_* definitions for many more functions than those for which<br>
> we have LIBBUILTIN definitions. This unnecessarily pessimizes code<br>
> generation around many of the remaining libm functions.<br>
><br>
> 2. Many of these functions can be marked as readnone so long as we<br>
> can neglect floating point exceptions. I've added a new specifier<br>
> 'T', which like 'e' for errno, marks the builtin as const so long as<br>
> trapping math is disabled. I've added a TrappingMath lang option to<br>
> track this.<br>
><br>
> 3. Disables -ftrapping-math by default. This is currently enabled by<br>
> default (following gcc), but this is silly. No system of which I'm<br>
> aware actually traps on floating-point exceptions by default (some<br>
> system-specific mechanism is necessary in order to enable SIGFPE<br>
> generation). In addition, in Clang, we currently don't support fenv<br>
> access. As a result, we can turn this off by default (and enable<br>
> better code generation around the functions for which we have<br>
> LIBBUILTIN definitions).<br>
><br>
> 4. Updates the CodeGen/libcall-declarations.c test case.<br>
><br>
> As a side effect, the existing libm LIBBUILTIN definitions that were<br>
> missing the 'n' (nothrow) specifier now have it.<br>
><br>
><br>
><br>
> For clarity, please split the patch into four separate patches:<br>
> reformatting, adding missing 'n' specifiers, the<br>
> trapping-math-related changes, and adding new LIBBUILTIN definitions<br>
> (preferably in that order). As it is, the Builtins.def changes are<br>
> completely unreviewable.<br>
<br>
</div></div>Unfortunately, because almost all of the LIBBUILTIN's in the patch are new, I don't think that splitting the patch will help that :( -- FWIW, I think that reviewing the Builtins.def changes means, in practice (either trusting me, or) going through the documentation on each of those functions to make sure that the flags are correct. I don't think that splitting the patch makes that easier either.<br>

<br>
Nevertheless, I'll split the changes and re-post as separate patches. Maybe I'm wrong ;)<br></blockquote><div><br></div><div>I didn't try to count how many were new... it looked like you were changing a bunch of existing ones, but if the balance is as you say, it might not be as useful as I'd hope.</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>
> Your changes could be substantially simplified if we don't allow<br>
> enabling -ftrapping-math; as you've noted, we don't honor it anyway.<br>
> What do you think?<br>
<br>
</div>I considered that, but I think that we might as well not make the situation worse for users who do enable trapping math for debugging purposes (as I do myself on occasion).</blockquote><div><br></div><div>Debugging how, exactly?</div>
<div> </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"> With these changes, we'll mark a lot more libm functions that could trap when exceptions are enabled as readnone (including some which never set errno), and if we ignore -ftrapping-math completely, we'll provide no way of indicating that those functions have side effects when we're interested in floating point exceptions.</blockquote>
<div><br></div><div>Providing -ftrapping-math as an option is deceptive at best because clang will break code which expects us to honor -ftrapping-math, even if it doesn't even use any library calls.</div><div><br></div>
<div>-Eli</div></div></div></div>