This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 1/3] New make target gen-libm-inputs
- From: Joseph Myers <joseph at codesourcery dot com>
- To: Siddhesh Poyarekar <siddhesh dot poyarekar at linaro dot org>
- Cc: <libc-alpha at sourceware dot org>
- Date: Fri, 11 Dec 2015 13:00:25 +0000
- Subject: Re: [PATCH 1/3] New make target gen-libm-inputs
- Authentication-results: sourceware.org; auth=none
- References: <20151211101911 dot GA2372 at linaro-laptop dot intra dot reserved-bit dot com>
On Fri, 11 Dec 2015, Siddhesh Poyarekar wrote:
> Hi,
>
> Create a new target gen-libm-inputs to generate auto-libm-test-out
> from auto-libm-test-in and a newly generated
> auto-libm-test-in-benchtests. The latter file is now generated from
> benchmark inputs in benchtests/*-inputs for all math functions that
> are included in the benchmarks. This includes a few thousand inputs
> for each of the functions to the test, effectively testing all paths
> of the major functions. I discovered the need for this when when
> patching sincos (patchset coming up soon) where the current test
> inputs did not catch a bug.
I don't think adding thousands of tests like that is a good idea (unless
function implementations genuinely have thousands of branches). Rather,
you should identify a minimal set of test inputs that actually improve
branch coverage or increase observed ulps, and only add those inputs to
auto-libm-test-in.
I'm also wary of a makefile rule like this because it's likely that MPFR
or MPC versions newer than the system ones are required (for example, as
soon as MPC 1.1 comes out with the performance improvements for mpc_sin
that went in two years ago but still haven't appeared in a release, I
intend to move the remaining csin tests to auto-libm-test-in, which will
mean regeneration may take over 10 minutes when using older MPC versions).
> Patch 2/3 and 3/3 are regenerated files, the first being regenerated
> auto-libm-test-out and the other being the x86_64 libm-test-ulps. The
> ulp update is trivial but the auto-libm-test-out update is massive
> (136MB), which is why I have left both out and will be pushing as
> obvious once this patch is approved.
I think that size of auto-libm-test-out illustrates why adding all the
benchmark inputs would be excessive.
(It might get that big if the MPC performance issues for complex inverse
trig and hyperbolic functions are fixed so those tests can move to
auto-libm-test-* - in general, each test input line for a complex function
test generates a lot more output than for a real function test - but then
it might be a good idea to split auto-libm-test-out, and the test-* tests
that get run by the makefiles, by function first; and for complex
functions and functions with multiple real arguments, generally there are
more cases to cover and so larger test size is better justified.)
> +# Very simple target. We don't really care for any glibc configuration to
> +# affect this. TODO: add configure check to determine if MPC, MPFR development
> +# libraries are available.
> +$(objpfx)gen-auto-libm-tests: $(gen-auto-libm-tests-modules:%=%.c)
> + $(CC) -o $@ $^ -O2 -std=gnu11 -Wall -Wextra -lmpc -lmpfr -lgmp
Of course this should be BUILD_CC.
> FUNC_mpfr_ff_f ("pow", mpfr_pow, false),
> + FUNC_mpfr_f_f ("rint", mpfr_erf, false),
> FUNC_mpfr_f_f ("sin", mpfr_sin, false),
> FUNC ("sincos", ARGS1 (type_fp), RET2 (type_fp, type_fp), false, false,
> false, CALC (mpfr_f_11, mpfr_sin_cos)),
No. If you want to test extra functions, add the support required to do
so properly, not hacking things up to accept test inputs whose outputs are
both meaningless and unused.
--
Joseph S. Myers
joseph@codesourcery.com