[LA64_DYNAREC] Improve x87 long double compare by yzewei · Pull Request #4142 · ptitSeb/box64

yzewei · 2026-07-28T12:35:19Z

Improved x87 long double comparisons on LA64 dynarec.

Fixed long double classification issues, such as NumPy's isinf/isfinite operations on np.longdouble values.

before

BOX64_DYNAREC=1 BOX64_LOG=0 PYTHONFAULTHANDLER=1  box64  python3 -X faulthandler - <<'PY'
import numpy as np                                                      
for name, val in [("+inf", np.inf), ("-inf", -np.inf), ("nan", np.nan)]:
    v = np.array([val], dtype="g")[0]
    print(name, bool(np.isinf(v)), bool(np.isfinite(v)), bool(np.isnan(v)))
PY
[BOX64] Box64 loongarch64 v0.4.3 with Dynarec built on Jul 28 2026 07:23:13
+inf False True False
-inf False True False
nan False False True

after

BOX64_DYNAREC=1 BOX64_LOG=0 PYTHONFAULTHANDLER=1  box64  python3 -X faulthandler - <<'PY'
import numpy as np                                                      
for name, val in [("+inf", np.inf), ("-inf", -np.inf), ("nan", np.nan)]:
    v = np.array([val], dtype="g")[0]
    print(name, bool(np.isinf(v)), bool(np.isfinite(v)), bool(np.isnan(v)))
PY
[BOX64] Box64 loongarch64 v0.4.3 with Dynarec built on Jul 28 2026 08:23:30
+inf True False False
-inf True False False
nan False False True

ptitSeb · 2026-07-28T13:06:21Z

                v2 = x87_get_st(dyn, ninst, x1, x2, nextop & 7, X87_COMBINE(0, nextop & 7));
+                MOVFR2GR_D(x6, v1);
+                MOVFR2GR_D(x7, v2);
+                x87_fcomi_fpu_ld(dyn, ninst, x1, x2, x3, x4, x5, x6, x7, nextop & 7);


same question here abotut having this optionnal?

ptitSeb · 2026-07-28T13:07:26Z

@ksco what do you think about those x87 tests: should they be optionnal or is it ok to have them all the time?

ksco · 2026-07-28T13:10:20Z

Absolutely optional. I need to find some time to review this PR, but not now, for sure.

yzewei · 2026-07-29T01:26:19Z

The change should take effect when FASTROUND=0. As for x87_clear_fpu_ld, the single branch is not responsible for maintaining ld, so it should indeed be removed.

ksco

I haven't finished the review, but this already confuses me. Could you please explain how this works in detail and what it is for? The NumPy example did not convince me, because it's a simple example that just shows that Box64 does not support long double, which is known. Does adding the exact support for this crafted example help any real-world program?

(This question holds for the interpreter change #4107 too, since that was already merged, maybe @ptitSeb has opinions too?)

ksco · 2026-07-29T03:11:27Z

+    MAYUSE(st);
+    MAYUSE(chs);
+
+#ifndef HAVE_LD80BITS


These helper functions update the simulated raw 80-bit shadow layout when HAVE_LD80BITS is unavailable, for example:

fpu_ld_t.ld.l.lower / upper

fpu_ld_t.uref

When HAVE_LD80BITS is available, fpu_ld_t stores a raw long double type, and the shadow layout does not need to be manually updated.

It makes no sense to enable HAVE_LD80BITS on a LoongArch build.

It makes no sense to enable HAVE_LD80BITS on a LoongArch build.

=.= That's true.

ksco · 2026-07-29T03:11:57Z

+    MAYUSE(a);
+    MAYUSE(b);
+
+#ifndef HAVE_LD80BITS


ksco · 2026-07-29T03:29:17Z

+        ADDI_D(addr, addr, st - dyn->lsx.x87stack);
+    ANDI(addr, addr, 7);
+
+    MOV32w(tmp, sizeof(fpu_ld_t));


This is a compile-time constant (64). Do we need to use MOV32 and MUL_D?

SLLI_D(addr, addr, 6);

ksco · 2026-07-29T03:31:09Z

+    ld80_get_addr(dyn, ninst, s1, s4, a);
+    ld80_get_addr(dyn, ninst, s2, s4, b);
+
+    for (int i = 0; i < sizeof(fpu_ld_t); i += 2) {


I don't understand, why the step here is 2?

I'm concerned that fpu_ld_t is a packed struct of #pragma pack(push, 1), and the uint64_t field inside is not guaranteed to be 8-byte aligned. Using half-word load/store operations can avoid unaligned 64-bit accesses and is also less expensive than copying byte by byte.

ksco · 2026-07-29T03:32:08Z

                // swap the cache value, not the double value itself :p
                x87_get_st(dyn, ninst, x1, x2, nextop & 7, X87_ST(nextop & 7));
                x87_get_st(dyn, ninst, x1, x2, 0, X87_ST0);
+                x87_swap_fpu_ld(dyn, ninst, x1, x2, x3, x4, 0, nextop & 7);


Wouldn't this need to be optional too?

ptitSeb · 2026-07-29T07:40:28Z

I wont merge this PR for current release anyway. This will wait for the next dev. cycle.

yzewei · 2026-07-29T09:36:34Z

I haven't finished the review, but this already confuses me. Could you please explain how this works in detail and what it is for? The NumPy example did not convince me, because it's a simple example that just shows that Box64 does not support long double, which is known. Does adding the exact support for this crafted example help any real-world program?

(This question holds for the interpreter change #4107 too, since that was already merged, maybe @ptitSeb has opinions too?)

The NumPy case does not require full long double arithmetic support, and this PR is not trying to add that.

The concrete issue is that, on targets without HAVE_LD80BITS, Box64 already keeps an emulated raw x87 80-bit shadow for some x87/long-double state. In the LA64 dynarec path, some x87 stack/sign operations update the cached ST double value, but the raw 80-bit shadow can remain stale. Later x87 compare/classification instructions may then observe the stale raw shadow.

This is visible in real NumPy APIs, not only in a crafted unit test. For example, longdouble classification/comparison paths such as np.isinf, np.isfinite, np.isnan, and longdouble division corner cases expect inf/NaN classification to be coherent.

So far, NumPy is the only real-world workload where I have verified this directly. But the underlying issue is not NumPy-specific: it is a consistency problem between the LA64 dynarec cached ST value and Box64's existing emulated raw x87 80-bit shadow.

I can update the PR description to make this scope clearer: it does not implement full long double support; it only keeps the existing raw x87 shadow coherent for the affected dynarec x87 operations.

Use Box64's existing raw x87 80-bit shadow for FCOMI/FUCOMI/ FCOMIP/FUCOMIP when BOX64_DYNAREC_FASTROUND=0, improving long double comparison accuracy. Keep the raw shadow coherent for FXCH and sign-only FABS/FCHS operations, fixing cases where values such as LDBL_MAX collapse to double infinity and break isinf/isfinite-style checks. Signed-off-by: Zewei Yang <yangzewei@loongson.cn>

ptitSeb reviewed Jul 28, 2026

View reviewed changes

Comment thread src/dynarec/la64/dynarec_la64_d9.c Outdated

ptitSeb reviewed Jul 28, 2026

View reviewed changes

Comment thread src/dynarec/la64/dynarec_la64_db.c Outdated

ptitSeb reviewed Jul 28, 2026

View reviewed changes

Comment thread src/dynarec/la64/dynarec_la64_df.c Outdated

ptitSeb requested a review from ksco July 28, 2026 13:07

yzewei force-pushed the x87_dynarec_la branch from 0836264 to 3a6ce63 Compare July 29, 2026 01:27

ksco requested changes Jul 29, 2026

View reviewed changes

yzewei force-pushed the x87_dynarec_la branch 2 times, most recently from 0f640d1 to 3ece8b6 Compare July 30, 2026 01:04

yzewei force-pushed the x87_dynarec_la branch from 3ece8b6 to bad6636 Compare July 30, 2026 01:48

Uh oh!

Conversation

yzewei commented Jul 28, 2026

before

after

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ptitSeb commented Jul 28, 2026

Uh oh!

ksco commented Jul 28, 2026

Uh oh!

yzewei commented Jul 29, 2026

Uh oh!

ksco left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yzewei Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ptitSeb commented Jul 29, 2026

Uh oh!

yzewei commented Jul 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yzewei Jul 29, 2026 •

edited

Loading