diff mbox series

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

Message ID 20240321164325.539976-2-adhemerval.zanella@linaro.org
State Superseded
Headers show
Series Math static build fixes | expand

Commit Message

Adhemerval Zanella Netto March 21, 2024, 4:43 p.m. UTC
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.
---
 math/Makefile               | 104 +++++++++++++++++++++++++++++++++++-
 math/test-double-static.h   |   1 +
 math/test-float-static.h    |   1 +
 math/test-float128-static.h |   1 +
 math/test-float32-static.h  |   1 +
 math/test-float32x-static.h |   1 +
 math/test-float64-static.h  |   1 +
 math/test-float64x-static.h |   1 +
 math/test-ibm128-static.h   |   1 +
 math/test-ldouble-static.h  |   1 +
 10 files changed, 111 insertions(+), 2 deletions(-)
 create mode 100644 math/test-double-static.h
 create mode 100644 math/test-float-static.h
 create mode 100644 math/test-float128-static.h
 create mode 100644 math/test-float32-static.h
 create mode 100644 math/test-float32x-static.h
 create mode 100644 math/test-float64-static.h
 create mode 100644 math/test-float64x-static.h
 create mode 100644 math/test-ibm128-static.h
 create mode 100644 math/test-ldouble-static.h

Comments

Joseph Myers March 21, 2024, 11:35 p.m. UTC | #1
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.
Florian Weimer March 22, 2024, 6:46 a.m. UTC | #2
* 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.

Thanks,
Florian
Adhemerval Zanella Netto March 22, 2024, 2:14 p.m. UTC | #3
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

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.

I can add simple linking tests for this specific failure, and I also think
that reword all the static ABI check for this issues in a bit too much
for this patchset.
Florian Weimer March 22, 2024, 3:51 p.m. UTC | #4
* 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
>
> 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.

Thanks,
Florian
Adhemerval Zanella Netto March 22, 2024, 5:46 p.m. UTC | #5
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
>>
>> 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.
H.J. Lu March 25, 2024, 1:34 p.m. UTC | #6
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.

> >> 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.
Adhemerval Zanella Netto March 25, 2024, 2:13 p.m. UTC | #7
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.
  
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.
H.J. Lu March 25, 2024, 2:25 p.m. UTC | #8
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.

> 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.
Adhemerval Zanella Netto March 25, 2024, 2:29 p.m. UTC | #9
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.
> 
> 
>
H.J. Lu March 25, 2024, 2:52 p.m. UTC | #10
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?
Adhemerval Zanella Netto March 25, 2024, 5:41 p.m. UTC | #11
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). 

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.
H.J. Lu March 26, 2024, 1:40 p.m. UTC | #12
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.
Joseph Myers March 26, 2024, 5:37 p.m. UTC | #13
On Tue, 26 Mar 2024, H.J. Lu wrote:

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

That becomes several hundred GB for build-many-glibcs.py (albeit that 
build directories are normally deleted for each configuration after 
testing of that configuration completes, so not all necessarily exist at 
the same time).
Adhemerval Zanella Netto March 26, 2024, 5:55 p.m. UTC | #14
On 26/03/24 14:37, Joseph Myers wrote:
> On Tue, 26 Mar 2024, H.J. Lu wrote:
> 
>> The current glibc build takes 2.7G on x86-64.  I don't see another 4GB is
>> a big issue.
> 
> That becomes several hundred GB for build-many-glibcs.py (albeit that 
> build directories are normally deleted for each configuration after 
> testing of that configuration completes, so not all necessarily exist at 
> the same time).
> 

My plan is to send a new patchset version, with static fixes for some ABIs,
but without enabling *all* the tests to be built statically. I think we can 
discuss this in a subsequent patch if this actually is worth.
H.J. Lu March 26, 2024, 5:59 p.m. UTC | #15
On Tue, Mar 26, 2024 at 10:55 AM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 26/03/24 14:37, Joseph Myers wrote:
> > On Tue, 26 Mar 2024, H.J. Lu wrote:
> >
> >> The current glibc build takes 2.7G on x86-64.  I don't see another 4GB is
> >> a big issue.
> >
> > That becomes several hundred GB for build-many-glibcs.py (albeit that
> > build directories are normally deleted for each configuration after
> > testing of that configuration completes, so not all necessarily exist at
> > the same time).
> >
>
> My plan is to send a new patchset version, with static fixes for some ABIs,
> but without enabling *all* the tests to be built statically. I think we can
> discuss this in a subsequent patch if this actually is worth.

If disk space is a concern, we can add a configure option,
---enable-static-math-tests, to enable static math tests.
diff mbox series

Patch

diff --git a/math/Makefile b/math/Makefile
index 79ef4ebb65..aef9cec1a1 100644
--- a/math/Makefile
+++ b/math/Makefile
@@ -274,8 +274,10 @@  endif
 
 libm-vec-tests = $(addprefix test-,$(libmvec-tests))
 libm-test-support = $(foreach t,$(test-types),libm-test-support-$(t))
-test-extras += $(libm-test-support)
-extra-test-objs += $(addsuffix .o, $(libm-test-support))
+libm-test-support-static = $(foreach t,$(test-types),libm-test-support-$(t)-static)
+test-extras += $(libm-test-support) $(libm-test-support-static)
+extra-test-objs += $(addsuffix .o, $(libm-test-support)) \
+		   $(addsuffix .o, $(libm-test-support-static))
 libm-vec-test-wrappers = $(addsuffix -wrappers, $(libm-vec-tests))
 test-extras += $(libm-vec-test-wrappers)
 extra-test-objs += $(addsuffix .o, $(libm-vec-test-wrappers))
@@ -364,6 +366,64 @@  $(libm-test-c-narrow-obj): $(objpfx)libm-test%.c: libm-test%.inc \
 	$(make-target-directory)
 	$(PYTHON) gen-libm-test.py -c $< -a auto-libm-test-out$* -C $@
 
+
+libm-test-funcs-auto-static =
+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)
+
+libm-test-c-auto-static = $(foreach f,$(libm-test-funcs-auto-static),libm-test-$(f)-static.c)
+libm-test-c-noauto-static = $(foreach f,$(libm-test-funcs-noauto-static),libm-test-$(f)-static.c)
+libm-test-c-narrow-static = $(foreach f,$(libm-test-funcs-narrow-static),\
+				 libm-test-narrow-$(f)-static.c)
+generated += $(libm-test-c-auto-static) $(libm-test-c-noauto-static) $(libm-test-c-narrow-static)
+
+libm-tests-normal-static = $(foreach t,$(libm-tests-base-normal),\
+				$(foreach f,$(libm-test-funcs-all-static),\
+					    $(t)-$(f)-static))
+libm-tests-narrow-static = $(foreach t,$(libm-tests-base-narrow-static),\
+				$(foreach f,$(libm-test-funcs-narrow-static),\
+					    $(t)-$(f)-static))
+libm-tests-vector-static = $(foreach t,$(libmvec-tests-static),\
+				$(foreach f,$($(t)-funcs),test-$(t)-$(f)-static))
+libm-tests-static = $(libm-tests-normal-static) $(libm-tests-narrow-static) $(libm-tests-vector-static)
+libm-tests-for-type-static = $(foreach f,$(libm-test-funcs-all-static),\
+					 test-$(1)-$(f)-static test-i$(1)-$(f)-static) \
+			     $(filter test-$(1)-%,$(libm-tests-vector-static) \
+						  $(libm-tests-narrow-static))
+
+libm-tests.o += $(addsuffix .o,$(libm-tests-static))
+
+tests-static += $(libm-tests-static)
+generated += $(addsuffix .c,$(libm-tests)) \
+	     $(foreach t,$(test-types),libm-test-support-$(t)-static.c)
+
+libm-test-c-auto-obj-static = $(addprefix $(objpfx),$(libm-test-c-auto-static))
+libm-test-c-noauto-obj-static = $(addprefix $(objpfx),$(libm-test-c-noauto-static))
+libm-test-c-narrow-obj-static = $(addprefix $(objpfx),$(libm-test-c-narrow-static))
+
+# Use the same input test definitions for both dynamic and static tests.
+.SECONDEXPANSION:
+$(libm-test-c-noauto-obj-static): $(objpfx)libm-test%.c: libm-test$$(subst -static,,%).inc \
+							 gen-libm-test.py
+	$(make-target-directory)
+	$(PYTHON) gen-libm-test.py -c $< -a /dev/null -C $@
+
+.SECONDEXPANSION:
+$(libm-test-c-auto-obj-static): $(objpfx)libm-test%.c: libm-test$$(subst -static,,%).inc \
+						       gen-libm-test.py \
+						       auto-libm-test-out$$(subst -static,,%)
+	$(make-target-directory)
+	$(PYTHON) gen-libm-test.py -c $< -a auto-libm-test-out`echo $* | sed 's/-static//'` -C $@
+
+.SECONDEXPANSION:
+$(libm-test-c-narrow-obj-static): $(objpfx)libm-test%.c: libm-test$$(subst -static,,%).inc \
+							 gen-libm-test.py \
+							 auto-libm-test-out$$(subst -static,,%)
+	$(make-target-directory)
+	$(PYTHON) gen-libm-test.py -c $< -a auto-libm-test-out`echo $* | sed 's/-static//'` -C $@
+
 # Tests for totalorder compat symbols reuse the table of tests as
 # processed by gen-libm-test.py, so add dependencies on the generated
 # .c files.
@@ -505,6 +565,18 @@  $(foreach t,$(libm-tests-normal),$(objpfx)$(t).c): $(objpfx)test-%.c:
 	  echo "#include <libm-test-$$func.c>"; \
 	) > $@
 
+$(foreach t,$(libm-tests-normal-static),$(objpfx)$(t).c): $(objpfx)test-%.c:
+	type_func=$*; \
+	type=$${type_func%%-*}; \
+	func=$${type_func#*-}; \
+	( \
+	  echo "#include <test-$$type.h>"; \
+	  echo "#include <test-math-exceptions.h>"; \
+	  echo "#include <test-math-errno.h>"; \
+	  echo "#include <test-math-scalar.h>"; \
+	  echo "#include <libm-test-$$func.c>"; \
+	) > $@
+
 $(foreach t,$(libm-tests-narrow),$(objpfx)$(t).c): $(objpfx)test-%.c:
 	type_pair_func=$*; \
 	type_pair=$${type_pair_func%-*}; \
@@ -539,6 +611,13 @@  $(foreach t,$(test-types),\
 	  echo "#include <libm-test-support.c>"; \
 	) > $@
 
+$(foreach t,$(test-types),\
+	    $(objpfx)libm-test-support-$(t)-static.c): $(objpfx)libm-test-support-%.c:
+	( \
+	  echo "#include <test-$*.h>"; \
+	  echo "#include <libm-test-support.c>"; \
+	) > $@
+
 $(addprefix $(objpfx), $(libm-tests.o)): $(objpfx)libm-test-ulps.h
 
 define o-iterator-doit
@@ -548,6 +627,13 @@  endef
 object-suffixes-left := $(libm-tests-base)
 include $(o-iterator)
 
+define o-iterator-doit
+$(foreach f,$(libm-test-funcs-all-static),\
+	    $(objpfx)$(o)-$(f)-static.o): $(objpfx)$(o)%.o: $(objpfx)libm-test%.c
+endef
+object-suffixes-left := $(libm-tests-base)
+include $(o-iterator)
+
 define o-iterator-doit
 $(foreach f,$(libm-test-funcs-narrow),\
 	    $(objpfx)$(o)-$(f).o): $(objpfx)$(o)%.o: \
@@ -563,6 +649,13 @@  endef
 object-suffixes-left := $(libm-tests-base-normal)
 include $(o-iterator)
 
+define o-iterator-doit
+$(foreach f,$(libm-test-funcs-all-static),\
+	    $(objpfx)$(o)-$(f)-static.o): CFLAGS += $(libm-test-no-inline-cflags)
+endef
+object-suffixes-left := $(libm-tests-base-normal)
+include $(o-iterator)
+
 define o-iterator-doit
 $(foreach f,$(libm-test-funcs-narrow),\
 	    $(objpfx)$(o)-$(f).o): CFLAGS += $(libm-test-no-inline-cflags)
@@ -584,6 +677,13 @@  endef
 object-suffixes-left := $(test-types)
 include $(o-iterator)
 
+define o-iterator-doit
+$(addprefix $(objpfx),\
+	    $(call libm-tests-for-type-static,$(o))): $(objpfx)libm-test-support-$(o)-static.o
+endef
+object-suffixes-left := $(test-types)
+include $(o-iterator)
+
 define o-iterator-doit
 $(objpfx)libm-test-support-$(o).o: CFLAGS += $(libm-test-no-inline-cflags)
 endef
diff --git a/math/test-double-static.h b/math/test-double-static.h
new file mode 100644
index 0000000000..d53f46819f
--- /dev/null
+++ b/math/test-double-static.h
@@ -0,0 +1 @@ 
+#include "test-double.h"
diff --git a/math/test-float-static.h b/math/test-float-static.h
new file mode 100644
index 0000000000..7834c9e1f1
--- /dev/null
+++ b/math/test-float-static.h
@@ -0,0 +1 @@ 
+#include "test-float.h"
diff --git a/math/test-float128-static.h b/math/test-float128-static.h
new file mode 100644
index 0000000000..5f8206456a
--- /dev/null
+++ b/math/test-float128-static.h
@@ -0,0 +1 @@ 
+#include "test-float128.h"
diff --git a/math/test-float32-static.h b/math/test-float32-static.h
new file mode 100644
index 0000000000..2df27d1ca0
--- /dev/null
+++ b/math/test-float32-static.h
@@ -0,0 +1 @@ 
+#include "test-float32.h"
diff --git a/math/test-float32x-static.h b/math/test-float32x-static.h
new file mode 100644
index 0000000000..62f78b49d8
--- /dev/null
+++ b/math/test-float32x-static.h
@@ -0,0 +1 @@ 
+#include "test-float32x.h"
diff --git a/math/test-float64-static.h b/math/test-float64-static.h
new file mode 100644
index 0000000000..807c174df1
--- /dev/null
+++ b/math/test-float64-static.h
@@ -0,0 +1 @@ 
+#include "test-float64.h"
diff --git a/math/test-float64x-static.h b/math/test-float64x-static.h
new file mode 100644
index 0000000000..a7801dbc10
--- /dev/null
+++ b/math/test-float64x-static.h
@@ -0,0 +1 @@ 
+#include "test-float64x.h"
diff --git a/math/test-ibm128-static.h b/math/test-ibm128-static.h
new file mode 100644
index 0000000000..b66a57050b
--- /dev/null
+++ b/math/test-ibm128-static.h
@@ -0,0 +1 @@ 
+#include "test-ibm128.h"
diff --git a/math/test-ldouble-static.h b/math/test-ldouble-static.h
new file mode 100644
index 0000000000..beabedb817
--- /dev/null
+++ b/math/test-ldouble-static.h
@@ -0,0 +1 @@ 
+#include "test-ldouble.h"