Re: [PATCH 1/3] Cleanup __ieee754_sqrt(f/l)

[Joseph Myers <joseph at codesourcery dot com>]

On Tue, 26 Sep 2017, Wilco Dijkstra wrote:

> This patch series cleans up the many uses of  __ieee754_sqrt(f/l) in GLIBC.
> The goal is to enable GCC to do the inlining, and if this fails call the
> __ieee754_sqrt function.  This is done by internally declaring sqrt with asm
> redirects.  The compat symbols and sqrt wrappers need to disable the redirect.
> 
> This means targets are no longer forced to add a special inline for sqrt.

I don't think these patches are reviewable without each patch including a 
description of how it was tested.  Appropriate testing might be a full 
build-many-glibcs.py run for each patch in the series (but at least, you 
need to include testing for at least one configuration where these 
functions are not inlined, and at least one configuration with float128 
support).

>         * math/Makefile: Add -fno-math-errno, but build tests
>         with -fmath-errno.

This seems extremely fragile.  It looks like it's relying on options added 
with one variable coming after options added with another, which is a 
matter of implementation details of how the compilation commands are built 
up from different variables.

I think there should be a single variable variable that adds the required 
option in every case - always at most one of -fmath-errno and 
-fno-math-errno, with no file ever getting both - probably based on 
$(in-module), and that should then be used at top level alongside 
-frounding-math, rather than only in the math/ directory.

-- 
Joseph S. Myers
joseph@codesourcery.com