Re: [PATCH 1/3] New make target gen-libm-inputs

[Siddhesh Poyarekar <sid at reserved-bit dot com>]

On Fri, Dec 11, 2015 at 01:00:25PM +0000, Joseph Myers wrote:
> 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.

OK, then that will have to be a completely different workflow.  What
do you think about this: add systemtap probes to each branch point in
the libm code and then have a driver link against this libc and
identify an input for each branch.  These probe points get built in
only for the gen-libm-inputs target and should get invalidated on the
default target.

The downside I see is that the code is slightly harder to read due to
all of these probe points, but it shouldn't be that bad.

> 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).

That is why this rule is optional, i.e. at the descretion of
developers only.  That said, I could add a configure option to specify
which MPC and MPFR you want to build against.  Would that be
acceptable?

> I think that size of auto-libm-test-out illustrates why adding all the 
> benchmark inputs would be excessive.

Fair enough; it worked for me because I found bugs, but I see what you
mean.

> > +# 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.

Sorry, I don't even remember how I came up with that bit.

Siddhesh