----- Original Message ----- > > On Fri, Sep 6, 2013 at 8:27 AM, Hal Finkel < [email protected] > 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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
