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

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

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.

>> I don't have a strong opinion in fact, and I am ok on changing to Joseph's
>> suggestion to check minimal tests to check for static linking support.
> 
> 
>