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 10:41 AM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 25/03/24 11:52, H.J. Lu wrote:
> > 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
> > ...
> >
>
> These are not tests, but rather the complex absolute (cabs/cabsf/etc.)
> that are autogenerated from the cabs_template.c.
>
> > Can we also generate
> >
> > math/cabs-static.c
> > ...
> >
> > and add them to libm tests?
> >
>
> And it should be simple to add static tests using these new rules:
>
> diff --git a/math/Makefile b/math/Makefile
> index ad7fd25995..67618a4385 100644
> --- a/math/Makefile
> +++ b/math/Makefile
> @@ -368,6 +368,7 @@ $(libm-test-c-narrow-obj): $(objpfx)libm-test%.c: libm-test%.inc \
>
>
>  libm-test-funcs-auto-static = \
> +  cabs \
>    exp10 \
>    # libm-test-funcs-auto-static
>  libm-test-funcs-noauto-static = \
>
> x86_64-linux-gnu$ file math/test-*-cabs-static
> math/test-double-cabs-static:   ELF 64-bit LSB pie executable, x86-64, version 1 (GNU/Linux), static-pie linked, for GNU/Linux 3.2.0, with debug_info, not stripped
> math/test-float-cabs-static:    ELF 64-bit LSB pie executable, x86-64, version 1 (GNU/Linux), static-pie linked, for GNU/Linux 3.2.0, with debug_info, not stripped
> math/test-float128-cabs-static: ELF 64-bit LSB pie executable, x86-64, version 1 (GNU/Linux), static-pie linked, for GNU/Linux 3.2.0, with debug_info, not stripped
> math/test-float32-cabs-static:  ELF 64-bit LSB pie executable, x86-64, version 1 (GNU/Linux), static-pie linked, for GNU/Linux 3.2.0, with debug_info, not stripped
> math/test-float32x-cabs-static: ELF 64-bit LSB pie executable, x86-64, version 1 (GNU/Linux), static-pie linked, for GNU/Linux 3.2.0, with debug_info, not stripped
> math/test-float64-cabs-static:  ELF 64-bit LSB pie executable, x86-64, version 1 (GNU/Linux), static-pie linked, for GNU/Linux 3.2.0, with debug_info, not stripped
> math/test-float64x-cabs-static: ELF 64-bit LSB pie executable, x86-64, version 1 (GNU/Linux), static-pie linked, for GNU/Linux 3.2.0, with debug_info, not stripped
> math/test-ldouble-cabs-static:  ELF 64-bit LSB pie executable, x86-64, version 1 (GNU/Linux), static-pie linked, for GNU/Linux 3.2.0, with debug_info, not stripped
>
> To add all the autogenerated tests, it is a matter to do something like:
>
> diff --git a/math/Makefile b/math/Makefile
> index ad7fd25995..b9f1b839ae 100644
> --- a/math/Makefile
> +++ b/math/Makefile
> @@ -307,7 +307,7 @@ libm-test-funcs-noauto = canonicalize ceil cimag conj copysign cproj creal \
>                          nextup remainder remquo rint round roundeven scalb \
>                          scalbln scalbn setpayload setpayloadsig signbit \
>                          significand totalorder totalordermag trunc ufromfp \
> -                        ufromfpx compat_totalorder compat_totalordermag
> +                        ufromfpx
>  libm-test-funcs-compat = compat_totalorder compat_totalordermag
>  libm-test-funcs-narrow = add div fma mul sqrt sub
>  libm-test-funcs-all = $(libm-test-funcs-auto) $(libm-test-funcs-noauto)
> @@ -368,13 +368,15 @@ $(libm-test-c-narrow-obj): $(objpfx)libm-test%.c: libm-test%.inc \
>
>
>  libm-test-funcs-auto-static = \
> -  exp10 \
> +  $(libm-test-funcs-auto) \
>    # libm-test-funcs-auto-static
>  libm-test-funcs-noauto-static = \
> -  fmod \
> +  $(libm-test-funcs-noauto) \
>    # libm-test-funcs-noauto-static
>  libm-test-funcs-compat-static =
> -libm-test-funcs-narrow-static =
> +libm-test-funcs-narrow-static = \
> +  $(libm-test-funcs-narrow) \
> +  # libm-test-funcs-narrow-static
>  libm-test-funcs-all-static = $(libm-test-funcs-auto-static) $(libm-test-funcs-noauto-static)
>
>  libm-test-c-auto-static = $(foreach f,$(libm-test-funcs-auto-static),libm-test-$(f)-static.c)
>
> (this won't work because of another missing symbol, __isnanf128, but which
> is also simple to fix).

Another reason to do it.

> The approach might be time consuming, since it requires more disk space and
> cpu use.  To just build the tests on my machine (Ryzen 5900, 12c) it takes
> about 17s, however the extra disk space is quite high:
>
> x86_64-linux-gnu$ stat -c "%s" math/test-*-static | awk '{ total += $0 }; END { print total }' | numfmt --to iec --format "%8.4f"
>  3,6716G
>
> I could get a smaller with '-fdata-sections -ffunction-sections -Wl,-gc-sections' (3,5940G),
> but it is still a somewhat amount of extra disk space for testing.

The current glibc build takes 2.7G on x86-64.  I don't see another 4GB is
a big issue.

-- 
H.J.