[PATCH] Trapping math and libfuncs improvements

[Hal Finkel]

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

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.

Nevertheless, I'll split the changes and re-post as separate patches. Maybe I'm wrong ;)

> 
> 
> Your changes could be substantially simplified if we don't allow
> enabling -ftrapping-math; as you've noted, we don't honor it anyway.
> What do you think?

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). 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.

Thanks again,
Hal

> 
> 
> -Eli

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory