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

[Joseph Myers <joseph@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 2835 bytes --]

On Wed, 14 Mar 2018, Wilco Dijkstra wrote:

> Maybe we can agree on a useful subset in the script - the number of 
> variants for some ISAs is ridiculous, do we really need 24 variants for 
> MIPS? I suppose we can also remove tile and a few others like sh given 
> the recent discussions of obsoleting those ports?

The point is to cover all ABIs supported by glibc (of which there are 24 
for MIPS), and then useful non-ABI variants that significantly affect the 
code that gets built.

If an architecture port is removed from glibc, the same commit removing it 
from glibc should of course remove it from build-many-glibcs.py.  If tile 
does get removed from the Linux kernel I think at that point we should 
remove it from glibc.  I'm not aware of any evidence that sh is going to 
be removed from the Linux kernel or any proposals to remove it from glibc 
(although it does need a maintainer in glibc).

> > What execution testing have you done?  It would be a good idea for that to 
> > include at least one configuration where sqrt is not inlined by the 
> > compiler, and also to include 32-bit x86 to make sure the special-case 
> > wrappers there (to avoid double rounding) continue to work as expected.
> 
> I run on AArch64 and x64 when it is relevant. Finding a config that 
> doesn't inline sqrt may be difficult, since pretty much all ISAs have a 
> sqrt instruction.

Soft-float configurations (e.g. soft-float ARM) don't inline sqrt.

> We build the testsuite explicitly with -fmath-errno so there will be 
> calls to sqrt. Given the include path is incorrect when running the 
> testsuite (it uses internal paths rather than external ones), we get 

_ISOMAC should be defined for building the testsuite except for a few 
tests listed in tests-internal.  The include/ headers should, when _ISOMAC 
is defined, do nothing except include the corresponding public header 
(which is required so it gets found at all, since directories such as 
math/ contain the public headers but aren't in the include search path 
when building source files from other glibc subdirectories).

> redirection to __ieee754_sqrt which is not exported outside of GLIBC. 
> Since the include path issue is a separate bug I just ensure the sqrt 
> benchmark gets the right include - when it is fixed we can easily change 
> it back.

If the benchmarks are compiled without _ISOMAC being defined, that's the 
bug that should be fixed.  Changing them to use math/math.h is definitely 
wrong.

> Btw we'll also need to create some real inputs and decide what we want 
> to test: the inlined sqrt instruction or the sqrt call in GLIBC (which 
> will usually be the sqrt instruction), or the sqrt emulation...

The benchmarks are supposed to test the function call in glibc (and thus 
use -fno-builtin).

-- 
Joseph S. Myers
joseph@codesourcery.com