[PATCH 0/1] MR5276: musl: Fix limiting the float precision in intermediates.
musl itself expects to be configured to compile with either -ffloat-store or -fexcess-precision=standard - but when imported into Wine, those flags aren't used. This seems to be essential for getting reasonable precision from some math functions such as exp2() - without the expected precision truncation, the output value of exp2() can be off by as much as 0.2% in some cases. As Wine doesn't build the musl sources with those flags, use volatile to force storing/reloading floats in order to limit their intermediate precision, as musl expects. Only do this on i386 where this is known be required. This fixes https://bugs.winehq.org/show_bug.cgi?id=56372. Apparently this issue only appears when compiled with GCC; with Clang, this already works as expected. Signed-off-by: Martin Storsjö <martin(a)martin.st> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5276
From: Martin Storsjö <martin(a)martin.st> musl itself expects to be configured to compile with either -ffloat-store or -fexcess-precision=standard - but when imported into Wine, those flags aren't used. This seems to be essential for getting reasonable precision from some math functions such as exp2() - without the expected precision truncation, the output value of exp2() can be off by as much as 0.2% in some cases. As Wine doesn't build the musl sources with those flags, use volatile to force storing/reloading floats in order to limit their intermediate precision, as musl expects. Only do this on i386 where this is known be required. This fixes https://bugs.winehq.org/show_bug.cgi?id=56372. Apparently this issue only appears when compiled with GCC; with Clang, this already works as expected. Signed-off-by: Martin Storsjö <martin(a)martin.st> --- libs/musl/src/internal/libm.h | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/libs/musl/src/internal/libm.h b/libs/musl/src/internal/libm.h index a1e9bc08716..afa987d9c7d 100644 --- a/libs/musl/src/internal/libm.h +++ b/libs/musl/src/internal/libm.h @@ -104,16 +104,25 @@ static int32_t converttoint(double_t); /* Evaluate an expression as the specified type. With standard excess precision handling a type cast or assignment is enough (with -ffloat-store an assignment is required, in old compilers argument - passing and return statement may not drop excess precision). */ + passing and return statement may not drop excess precision). + + If compiled without -ffloat-store or -fexcess-precision=standard, + an extra volatile qualifier here will force limiting the precision. */ static inline float eval_as_float(float x) { +#ifdef __i386__ + volatile +#endif float y = x; return y; } static inline double eval_as_double(double x) { +#ifdef __i386__ + volatile +#endif double y = x; return y; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5276
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=143944 Your paranoid android. === debian11b (64 bit WoW report) === d2d1: Unhandled exception: page fault on read access to 0xffffffffffffffff in 64-bit code (0x00000072a85c34). secur32: schannel.c:538: Test failed: cert_cnt = 2
`exp2()` works incorrectly when 24-bit precision is used (`_control87(_PC_24, _MCW_PC)`). It's probably better to change the implementation so it's not affected by precision set on function call. I guess there's similar problem in other functions. I'm not sure what's the best way of addressing it (I guess it may be side effect of using SSE). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5276#note_64450
On Tue Mar 12 20:45:00 2024 +0000, Piotr Caban wrote:
`exp2()` works incorrectly when 24-bit precision is used (`_control87(_PC_24, _MCW_PC)`). It's probably better to change the implementation so it's not affected by precision set on function call. I guess there's similar problem in other functions. I'm not sure what's the best way of addressing it (I guess it may be side effect of using SSE). Ok, although I probably won't be volunteering to do that (you know better than me in which form you'd like it). I presume the previous implementation here didn't have these issues?
(I'm not sure how many of the other musl math functions that may have similar issues, this is the only one that did affect a notable test outcome for me so far. If the fix is easy and small enough, I would have suggested to backport it to further 9.0.x stable releases, as this issue was a notable regression when updating from 8.0.x to 9.0.x.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5276#note_64451
Fixed by !6668, closing this one. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5276#note_84985
This merge request was closed by Martin Storsjö. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5276
participants (3)
-
Martin Storsjö -
Marvin -
Piotr Caban (＠piotr)