[PATCH 0/11] MR2567: Bundle the musl math library

newer
[PATCH 0/1] MR2575: ws2_32/tests:...

older
[PATCH v2 0/3] MR2573: dinput: Set...

Alexandre Julliard (＠julliard)

3 Apr 2023 3 Apr '23

2:51 p.m.

The goal is to use the bundling mechanism to share the musl math library code between msvcrt and ntdll, and also with a future FPU emulation library. It should also make it easier to sync with upstream changes. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2567

Show replies by date

Alexandre Julliard

3 Apr 3 Apr

2:51 p.m.

New subject: [PATCH 11/11] msvcrt: Use the copysign()/copysignf() implementation from the bundled musl library.

From: Alexandre Julliard <julliard(a)winehq.org> --- dlls/crtdll/crtdll.spec | 2 +- dlls/msvcr100/msvcr100.spec | 4 ++-- dlls/msvcr110/msvcr110.spec | 4 ++-- dlls/msvcr120/msvcr120.spec | 10 +++++----- dlls/msvcr70/msvcr70.spec | 2 +- dlls/msvcr71/msvcr71.spec | 2 +- dlls/msvcr80/msvcr80.spec | 4 ++-- dlls/msvcr90/msvcr90.spec | 4 ++-- dlls/msvcrt/math.c | 26 -------------------------- dlls/msvcrt/msvcrt.spec | 4 ++-- dlls/msvcrtd/msvcrtd.spec | 2 +- dlls/ucrtbase/ucrtbase.spec | 10 +++++----- 12 files changed, 24 insertions(+), 50 deletions(-) diff --git a/dlls/crtdll/crtdll.spec b/dlls/crtdll/crtdll.spec index d5192ad7569..c66cea5bade 100644 --- a/dlls/crtdll/crtdll.spec +++ b/dlls/crtdll/crtdll.spec @@ -64,7 +64,7 @@ @ extern _commode_dll MSVCRT__commode @ cdecl _control87(long long) @ cdecl _controlfp(long long) -@ cdecl _copysign(double double) +@ cdecl _copysign(double double) copysign @ varargs _cprintf(str) @ extern _cpumode_dll CRTDLL__cpumode_dll @ cdecl _cputs(str) diff --git a/dlls/msvcr100/msvcr100.spec b/dlls/msvcr100/msvcr100.spec index 3e2839b3908..a42fe349cf5 100644 --- a/dlls/msvcr100/msvcr100.spec +++ b/dlls/msvcr100/msvcr100.spec @@ -733,8 +733,8 @@ @ cdecl _control87(long long) @ cdecl _controlfp(long long) @ cdecl _controlfp_s(ptr long long) -@ cdecl _copysign(double double) -@ cdecl -arch=!i386 _copysignf(float float) +@ cdecl _copysign(double double) copysign +@ cdecl -arch=!i386 _copysignf(float float) copysignf @ varargs _cprintf(str) @ stub _cprintf_l @ stub _cprintf_p diff --git a/dlls/msvcr110/msvcr110.spec b/dlls/msvcr110/msvcr110.spec index c23293ea658..2bccd8c6fe9 100644 --- a/dlls/msvcr110/msvcr110.spec +++ b/dlls/msvcr110/msvcr110.spec @@ -1081,8 +1081,8 @@ @ cdecl _control87(long long) @ cdecl _controlfp(long long) @ cdecl _controlfp_s(ptr long long) -@ cdecl _copysign(double double) -@ cdecl -arch=!i386 _copysignf(float float) +@ cdecl _copysign(double double) copysign +@ cdecl -arch=!i386 _copysignf(float float) copysignf @ varargs _cprintf(str) @ stub _cprintf_l @ stub _cprintf_p diff --git a/dlls/msvcr120/msvcr120.spec b/dlls/msvcr120/msvcr120.spec index 6eac1937227..ed0f69b49bc 100644 --- a/dlls/msvcr120/msvcr120.spec +++ b/dlls/msvcr120/msvcr120.spec @@ -1073,8 +1073,8 @@ @ cdecl _control87(long long) @ cdecl _controlfp(long long) @ cdecl _controlfp_s(ptr long long) -@ cdecl _copysign(double double) -@ cdecl _copysignf(float float) +@ cdecl _copysign(double double) copysign +@ cdecl _copysignf(float float) copysignf @ varargs _cprintf(str) @ stub _cprintf_l @ stub _cprintf_p @@ -2091,9 +2091,9 @@ @ stub conj @ stub conjf @ stub conjl -@ cdecl copysign(double double) _copysign -@ cdecl copysignf(float float) _copysignf -@ cdecl copysignl(double double) _copysign +@ cdecl copysign(double double) +@ cdecl copysignf(float float) +@ cdecl copysignl(double double) copysign @ cdecl cos(double) @ cdecl -arch=!i386 cosf(float) @ cdecl cosh(double) diff --git a/dlls/msvcr70/msvcr70.spec b/dlls/msvcr70/msvcr70.spec index 60fa9ed8693..cb3a92cf395 100644 --- a/dlls/msvcr70/msvcr70.spec +++ b/dlls/msvcr70/msvcr70.spec @@ -258,7 +258,7 @@ @ extern _commode MSVCRT__commode @ cdecl _control87(long long) @ cdecl _controlfp(long long) -@ cdecl _copysign(double double) +@ cdecl _copysign(double double) copysign @ varargs _cprintf(str) @ cdecl _cputs(str) @ cdecl _cputws(wstr) diff --git a/dlls/msvcr71/msvcr71.spec b/dlls/msvcr71/msvcr71.spec index 1aacd768d46..a559b5cc376 100644 --- a/dlls/msvcr71/msvcr71.spec +++ b/dlls/msvcr71/msvcr71.spec @@ -253,7 +253,7 @@ @ extern _commode MSVCRT__commode @ cdecl _control87(long long) @ cdecl _controlfp(long long) -@ cdecl _copysign(double double) +@ cdecl _copysign(double double) copysign @ varargs _cprintf(str) @ cdecl _cputs(str) @ cdecl _cputws(wstr) diff --git a/dlls/msvcr80/msvcr80.spec b/dlls/msvcr80/msvcr80.spec index cf885153e7a..06d0bdf82cd 100644 --- a/dlls/msvcr80/msvcr80.spec +++ b/dlls/msvcr80/msvcr80.spec @@ -397,8 +397,8 @@ @ cdecl _control87(long long) @ cdecl _controlfp(long long) @ cdecl _controlfp_s(ptr long long) -@ cdecl _copysign(double double) -@ cdecl -arch=!i386 _copysignf(float float) +@ cdecl _copysign(double double) copysign +@ cdecl -arch=!i386 _copysignf(float float) copysignf @ varargs _cprintf(str) @ stub _cprintf_l @ stub _cprintf_p diff --git a/dlls/msvcr90/msvcr90.spec b/dlls/msvcr90/msvcr90.spec index 87b31d1a048..d020de98a02 100644 --- a/dlls/msvcr90/msvcr90.spec +++ b/dlls/msvcr90/msvcr90.spec @@ -380,8 +380,8 @@ @ cdecl _control87(long long) @ cdecl _controlfp(long long) @ cdecl _controlfp_s(ptr long long) -@ cdecl _copysign(double double) -@ cdecl -arch=!i386 _copysignf(float float) +@ cdecl _copysign(double double) copysign +@ cdecl -arch=!i386 _copysignf(float float) copysignf @ varargs _cprintf(str) @ stub _cprintf_l @ stub _cprintf_p diff --git a/dlls/msvcrt/math.c b/dlls/msvcrt/math.c index 6dcd9b5ec5f..c0d9baab361 100644 --- a/dlls/msvcrt/math.c +++ b/dlls/msvcrt/math.c @@ -216,19 +216,6 @@ float CDECL _chgsignf( float num ) return u.f; } -/********************************************************************* - * _copysignf (MSVCRT.@) - * - * Copied from musl: src/math/copysignf.c - */ -float CDECL _copysignf( float x, float y ) -{ - union { float f; UINT32 i; } ux = { x }, uy = { y }; - ux.i &= 0x7fffffff; - ux.i |= uy.i & 0x80000000; - return ux.f; -} - /********************************************************************* * _nextafterf (MSVCRT.@) * @@ -5723,19 +5710,6 @@ int CDECL fesetround(int round_mode) #endif /* _MSVCR_VER>=120 */ -/********************************************************************* - * _copysign (MSVCRT.@) - * - * Copied from musl: src/math/copysign.c - */ -double CDECL _copysign( double x, double y ) -{ - union { double f; UINT64 i; } ux = { x }, uy = { y }; - ux.i &= ~0ull >> 1; - ux.i |= uy.i & 1ull << 63; - return ux.f; -} - /********************************************************************* * _finite (MSVCRT.@) */ diff --git a/dlls/msvcrt/msvcrt.spec b/dlls/msvcrt/msvcrt.spec index 6dcb0767c31..43259d7abad 100644 --- a/dlls/msvcrt/msvcrt.spec +++ b/dlls/msvcrt/msvcrt.spec @@ -367,8 +367,8 @@ @ cdecl _control87(long long) @ cdecl _controlfp(long long) @ cdecl _controlfp_s(ptr long long) -@ cdecl _copysign(double double) -@ cdecl -arch=!i386 _copysignf(float float) +@ cdecl _copysign(double double) copysign +@ cdecl -arch=!i386 _copysignf(float float) copysignf @ varargs _cprintf(str) # stub _cprintf_l(str ptr) # stub _cprintf_p(str) diff --git a/dlls/msvcrtd/msvcrtd.spec b/dlls/msvcrtd/msvcrtd.spec index 33696eba829..5cb09d92c74 100644 --- a/dlls/msvcrtd/msvcrtd.spec +++ b/dlls/msvcrtd/msvcrtd.spec @@ -242,7 +242,7 @@ @ extern _commode MSVCRT__commode @ cdecl _control87(long long) @ cdecl _controlfp(long long) -@ cdecl _copysign(double double) +@ cdecl _copysign(double double) copysign @ varargs _cprintf(str) @ cdecl _cputs(str) @ cdecl _creat(str long) diff --git a/dlls/ucrtbase/ucrtbase.spec b/dlls/ucrtbase/ucrtbase.spec index bc9dd2bcbc6..87a266198bd 100644 --- a/dlls/ucrtbase/ucrtbase.spec +++ b/dlls/ucrtbase/ucrtbase.spec @@ -237,8 +237,8 @@ @ cdecl _control87(long long) @ cdecl _controlfp(long long) @ cdecl _controlfp_s(ptr long long) -@ cdecl _copysign(double double) -@ cdecl _copysignf(float float) +@ cdecl _copysign(double double) copysign +@ cdecl _copysignf(float float) copysignf @ cdecl _cputs(str) @ cdecl _cputws(wstr) @ cdecl _creat(str long) @@ -2236,9 +2236,9 @@ @ stub conj @ stub conjf @ stub conjl -@ cdecl copysign(double double) _copysign -@ cdecl copysignf(float float) _copysignf -@ cdecl copysignl(double double) _copysign +@ cdecl copysign(double double) +@ cdecl copysignf(float float) +@ cdecl copysignl(double double) copysign @ cdecl cos(double) @ cdecl -arch=!i386 cosf(float) @ cdecl cosh(double) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2567

Piotr Caban (＠piotr)

7:34 p.m.

The patches are looking good for me but I wonder what's the plan for functions that are implemented differently in msvcrt and musl? I guess the plan is to leave separate implementation if it's completely different. What about functions that only differ in how errno is set or math_error is called? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2567#note_28746

Zebediah Figura (＠zfigura)

7:38 p.m.

Why can't we implement these in ntdll and delegate to that from msvcrt? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2567#note_28747

Alexandre Julliard (＠julliard)

7:42 p.m.

...

What about functions that only differ in how errno is set or math_error is called?

The plan is to make the corresponding change in the bundled musl code. We don't need it to be identical to upstream, only close enough to make merges possible. Ideally the changes would be upstreamed at some point, at least the ones setting errno. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2567#note_28748

Piotr Caban (＠piotr)

8:19 p.m.

On Mon Apr 3 20:19:05 2023 +0000, Zebediah Figura wrote:

...

Why can't we implement these in ntdll and delegate to that from msvcrt? There's much more math functions in C-runtime. In some functions there are differences regarding error handling. We also had some bugs when functions were forwarded to ntdll. I think it makes sense to import musl so the code can be shared.

-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2567#note_28776

Piotr Caban (＠piotr)

8:19 p.m.

New subject: [PATCH 0/11] MR2567: Bundle the musl math library - approved

This merge request was approved by Piotr Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2567

Zebediah Figura (＠zfigura)

8:35 p.m.

...

There's much more math functions in C-runtime.

Sure, but if the point of this patch series is to share code with ntdll, then "delegate what we can to ntdll and leave everything else in msvcrt" seems like it'd avoid any duplication.

...

In some functions there are differences regarding error handling.

But then we can't share the function implementation anyway, can we? Also, all of the functions this patch series touches don't seem to have any error handling. Maybe that's just oversight, though?

...

We also had some bugs when functions were forwarded to ntdll.

Are those bugs specific to forwards? E.g. could -import be used instead? Or explicitly importing in the code, which might allow more flexibility in error handling as well. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2567#note_28782

Piotr Caban (＠piotr)

4 Apr 4 Apr

7:57 a.m.

...

Sure, but if the point of this patch series is to share code with ntdll, then "delegate what we can to ntdll and leave everything else in msvcrt" seems like it'd avoid any duplication.

No, there are some helpers/coefficients that will need to be duplicated (I didn't check if it's true, maybe all of them will end only in one place). Anyway, this will basically split musl math library into 2 places which may cause some code duplication.

...

But then we can't share the function implementation anyway, can we?

We can export errno / some other helper from musl. We don't want to do it from ntdll. Especially taking into account that errno is not shared between different msvcrt versions. It may also need to be stored in thread data.

...

Also, all of the functions this patch series touches don't seem to have any error handling. Maybe that's just oversight, though?

No, in case of this functions it's probably correct (e.g. sin() is exported from ntdll and uses math_error/errno).

...

Are those bugs specific to forwards? E.g. could -import be used instead? Or explicitly importing in the code, which might allow more flexibility in error handling as well.

The bugs were specific to forwards. We didn't have -import at that point but I guess it might work. I like having musl in separate, easily updatable place. I hope we will not loose this ability by adding to many changes. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2567#note_28818

1066

Age (days ago)

1067

Last active (days ago)

List overview

Download

18 comments

4 participants

participants (4)

Alexandre Julliard
Alexandre Julliard (＠julliard)
Piotr Caban (＠piotr)
Zebediah Figura (＠zfigura)

[PATCH 0/11] MR2567: Bundle the musl math library

tags

participants (4)