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

[Joseph Myers <joseph at codesourcery dot com>]

On Thu, 21 Dec 2017, Wilco Dijkstra wrote:

> > (Direct calling of sqrtf128 with similar redirects to those for sqrt / 
> > sqrtf / sqrtl is also appropriate: GCC 8 has it as a built-in function so 
> > can inline calls with -fno-math-errno on powerpc64le for POWER9.  So the 
> > patch should include a sqrtf128 redirect if __HAVE_DISTINCT_FLOAT128.)
> 
> I can't get this to work, some targets don't like __float128, others 
> don't like _Float128 despite __HAVE_DISTINCT_FLOAT128 being defined. I 

It's not being defined, it's being 1 rather than 0 (#if, not #ifdef).  
You should use _Float128, not __float128; bits/floatn.h handles defining 
_Float128 as a typedef if necessary for older compilers.

> don't see any calls to sqrtf128, so I think it's best to leave this for 
> now.

It's used (more precisely, __ieee754_sqrtf128 is used, which ought to 
change to calling sqrtf128 to allow inlining on POWER9 with GCC 8, just as 
with the changes to other __ieee754_sqrt* calls) (a) via the #defines in 
sysdeps/ieee754/float128/float128_private.h before ldbl-128 files are 
included

#define __ieee754_sqrtl __ieee754_sqrtf128
#define __sqrtl __sqrtf128
#define sqrtl sqrtf128

(which I expect won't need changing in the __ieee754_sqrt* -> sqrt* 
changes, since they already handle all variants of the function name) and 
(b) via sysdeps/generic/math-type-macros.h defining

#define M_SQRT M_SUF (__ieee754_sqrt)

which is then used in various type-generic templates, which thus end up 
using __ieee754_sqrtf128 when built for float128 (and changing that M_SQRT 
definition will obviously result in all types, including float128, 
changing at the same time to use sqrt* instead of __ieee754_sqrt*).

> There are many calls to simple functions like floor, round etc that 
> aren't currently inlined at all, so inlining those seems like a more 
> useful thing to do right now. I've renamed the define to 
> NO_MATH_REDIRECT to make this simple.

All those functions will of course need to handle float128 versions 
exactly the same.  (The GCC patch to support floorf128 etc. as built-in 
functions is still pending review, but doing the glibc changes does not in 
any way depend on such GCC changes, it simply means that such GCC changes 
will result in better optimization in glibc.)

-- 
Joseph S. Myers
joseph@codesourcery.com