Re: [PATCH v2 1/6] math: Add support for auto static math tests

["H.J. Lu" <hjl.tools@gmail.com>]

On Mon, Mar 25, 2024 at 7:29 AM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 25/03/24 11:25, H.J. Lu wrote:
> > On Mon, Mar 25, 2024 at 7:13 AM Adhemerval Zanella Netto
> > <adhemerval.zanella@linaro.org> wrote:
> >>
> >>
> >>
> >> On 25/03/24 10:34, H.J. Lu wrote:
> >>> On Fri, Mar 22, 2024 at 10:46 AM Adhemerval Zanella Netto
> >>> <adhemerval.zanella@linaro.org> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 22/03/24 12:51, Florian Weimer wrote:
> >>>>> * Adhemerval Zanella Netto:
> >>>>>
> >>>>>> On 22/03/24 03:46, Florian Weimer wrote:
> >>>>>>> * Joseph Myers:
> >>>>>>>
> >>>>>>>> On Thu, 21 Mar 2024, Adhemerval Zanella wrote:
> >>>>>>>>
> >>>>>>>>> It basically copy the already in place rules for dynamic tests
> >>>>>>>>> for auto-generated math tests for all support types.  To avoid
> >>>>>>>>> the need to duplicate .inc files, a .SECONDEXPANSION rules is
> >>>>>>>>> adeed for the gen-libm-test.py generation.
> >>>>>>>>
> >>>>>>>> Running the autogenerated tests seems overly complicated when the goal is
> >>>>>>>> simply to verify that linking a call succeeds.
> >>>>>>>
> >>>>>>> Right.  And I would prefer if we could mark compat/otherwise non-static
> >>>>>>> symbols in the ABI lists and use those for testing static linking.
> >>>>>>>
> >>>>>>
> >>>>>> That was my first approach, but then as an experiment I enabled static
> >>>>>> build for most of math tests and unexpectedly it has shows some failures
> >>>>>> on x86_64:
> >>>>>>
> >>>>>> FAIL: math/test-float64x-acos
> >>>>>> FAIL: math/test-float64x-log10
> >>>>>> FAIL: math/test-float64x-log2
> >>>>>> FAIL: math/test-float64x-y0
> >>>>>> FAIL: math/test-float64x-y1
> >>>>>> FAIL: math/test-ldouble-acos
> >>>>>> FAIL: math/test-ldouble-log10
> >>>>>> FAIL: math/test-ldouble-log2
> >>>>>> FAIL: math/test-ldouble-y0
> >>>>>> FAIL: math/test-ldouble-y1
> >>>>>>
> >>>>>> $ cat math/test-float64x-acos.out
> >>>>>> testing _Float64x (without inline functions)
> >>>>>> Failure: acos (max_value): Exception "Overflow" set
> >>>>>> Failure: acos (-max_value): Exception "Overflow" set
> >>>>>> Failure: acos_downward (max_value): Exception "Overflow" set
> >>>>>> Failure: acos_downward (-max_value): Exception "Overflow" set
> >>>>>> Failure: acos_towardzero (max_value): Exception "Overflow" set
> >>>>>> Failure: acos_towardzero (-max_value): Exception "Overflow" set
> >>>>>> Failure: acos_upward (max_value): Exception "Overflow" set
> >>>>>> Failure: acos_upward (-max_value): Exception "Overflow" set
> >>>>>>
> >>>
> >>> This new static test only checks link failure.  It doesn't check if the static
> >>> implementation is correct.  We may not have more functional coverage
> >>> for static libm in the first static libm test patch.  But the first new static
> >>> libm tests should least expose one static libm failure on x86-64.
> >>
> >> The first patch is just a framework so we can selective add new static
> >> tests, I haven't hook all of the autogenerated tests because this would
> >> add more cpu and disk usage.
> >>
> >> And the test added on libm-test-funcs-*-static rules does check for
> >> the implementation, using the default math skeleton test (including
> >> ulp, rounding, exceptions, etc).
> >>
> >>>
> >>>>>> My plan was to eventually track down what might be happening here, and
> >>>>>> the currently autogenerated tests gave me a nice scaffolding to add coverage
> >>>>>> tests.
> >>>>>
> >>>>> Interesting.  On the other hand, getting --disable-shared to work and
> >>>>> just run the *entire* test suite could provide value, too.  The last
> >>>>> time we discussed this we weren't sure if we had static-specific
> >>>>> failures, but your example shows that we do.
> >>>>>
> >>>>
> >>>> The main problem imho is --disable-shared is essentially a maintainer
> >>>> option. Although some installed programs will be static linked, it is
> >>>> really useful on checking if static linking is really working as expected.
> >>>>
> >>>> And it also requires *another* build and check iteration, which duplicates
> >>>> the work required in most cases (since static libraries are still built
> >>>> on default for --enable-shared).  I tried to help a coworker on support the
> >>>> --disable-shared and I recall another potential issues was the resulting
> >>>> disk usage (and thus build requirements) was quite high due glibc poor
> >>>> organization on static build requirements.
> >>>>
> >>>> There also another complication where we will need to constantly add
> >>>> $(build-shared) and duplicate the CI work to ensure both configure
> >>>> builds are ok.
> >>>>
> >>>> So I really think we should phase-out --disable-shared and work towards
> >>>> on add more static build tests.
> >>>
> >>> Agreed.  We should add one static libm functional test to each libm
> >>> functional test.  With this, the static libm link tests won't be needed.
> >>
> >> For this patchset only added the required one to check for symbols that
> >> there were some regression with recent fixes. But it should be doable to
> >> hook all tests for all symbols, although it would require some more Makefile
> >> rules to hook the tgmath ones.
> >
> > The first patch should just add the functional tests for the missing static
> > libm functions with Makefile changes which can be extended to cover
> > other libm functions.
> >
>
> And it does on second patch exactly as you suggested:
>
> diff --git a/math/Makefile b/math/Makefile
> index aef9cec1a1..fbb2987248 100644
> --- a/math/Makefile
> +++ b/math/Makefile
> @@ -368,7 +368,9 @@ $(libm-test-c-narrow-obj): $(objpfx)libm-test%.c: libm-test%.inc \
>
>
>  libm-test-funcs-auto-static =
> -libm-test-funcs-noauto-static =
> +libm-test-funcs-noauto-static = \
> +  fmod \
> +  # libm-test-funcs-noauto-static
>  libm-test-funcs-compat-static =
>  libm-test-funcs-narrow-static =
>  libm-test-funcs-all-static = $(libm-test-funcs-auto-static) $(libm-test-funcs-noauto-static)
>
>
> If you check the build directory, it will have a test-<type>-fmod-static
> that would fail to build without the rest of the patch.
>

We already generate libm tests under math:

math/cabs.c
math/cabsf128.c
math/cabsf.c
math/cabsl.c
...

Can we also generate

math/cabs-static.c
...

and add them to libm tests?

-- 
H.J.