Re: [PATCH] Cleanup __ieee754_sqrt(f)

[Joseph Myers <joseph at codesourcery dot com>]

On Fri, 15 Sep 2017, Wilco Dijkstra wrote:

> This patch cleans up the many uses of  __ieee754_sqrt(f) in GLIBC.
> We can compile all of the math functions with -fno-math-errno which 

I don't think that's appropriate for compiling testcases that test errno 
setting (even if in fact the use of -fno-builtin means it works for them).  
That is, unlike -frounding-math which is OK globally, -fno-math-errno 
should be handled more like the other options that are used for building 
glibc itself but not tests.

> means GCC will inline sqrt as a single instruction.  This means targets
> are no longer forced to add a special inline, and the resulting code is
> better without asm() constructs.
> 
> I verified there is no diff in the math libs except if there are new opportunities
> for optimization (for example fewer callee-saves are needed in some
> functions and sqrt was CSEd in a few cases).

Did you test with build-many-glibcs.py?  I'd expect configurations that 
don't inline sqrt to get localplt failures from this because of 
__builtin_sqrt calls resulting in sqrt references (and, in such cases, it 
would seem optimal for internal calls within glibc to continue to go to 
__ieee754_sqrt not to the errno-setting wrapper, which might require 
declaring sqrt with asm ("__ieee754_sqrt") while maybe doing something to 
avoid this causing problems for the definitions of the wrappers 
themselves.

> I have left __ieee754_sqrtl alone for now, however this could also
> be removed in principle. Also given pretty much every target has

I'd expect __ieee754_sqrtl to be handled the same way.  __builtin_sqrtf128 
doesn't exist (except for powerpc64le in GCC 8) so I wouldn't expect using 
__builtin_sqrt in M_SQRT to work without appropriate overrides for 
float128.

-- 
Joseph S. Myers
joseph@codesourcery.com