This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v3 1/3] Cleanup __ieee754_sqrt(f/l)
- From: Joseph Myers <joseph at codesourcery dot com>
- To: Wilco Dijkstra <Wilco dot Dijkstra at arm dot com>
- Cc: "libc-alpha at sourceware dot org" <libc-alpha at sourceware dot org>, nd <nd at arm dot com>
- Date: Tue, 13 Mar 2018 22:45:59 +0000
- Subject: Re: [PATCH v3 1/3] Cleanup __ieee754_sqrt(f/l)
- Authentication-results: sourceware.org; auth=none
- References: <DB6PR0801MB2053DCCDB0C7A490A861AE8783D20@DB6PR0801MB2053.eurprd08.prod.outlook.com>
On Tue, 13 Mar 2018, Wilco Dijkstra wrote:
> Tested using buildmanyglibc (13 most relevant variants since it keeps running
> out of diskspace...)
What variants exactly?
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.
Why is the benchtests/sqrt-inputs change needed? I think it's correct for
benchmark inputs to refer to the include name as in user code, not to the
path within the glibc source tree, and certainly this test should not be
doing things differently from all the others.
Please repost this patch series with ChangeLog entries updated to reflect
the current patch contents so they don't confuse the review. The
ChangeLog entry for this patch fails to mention the sqrtf128 declaration
in include/math.h. The one for patch 2 includes changes to
math/w_sqrt*_compat.c and sysdeps/aaarch64 files that aren't actually
included in the patch. The one for patch 3 suggests that only some parts
of sysdeps/s390/fpu/bits/mathinline.h are removed, when actually that
patch (correctly) removes the whole file.
--
Joseph S. Myers
joseph@codesourcery.com