Skip to content

Conversation

asemarafa
Copy link

This pull request fixes the FILD qword instruction, which was producing incorrect floating-point values on platforms with doesnt have an 80bit long double like M1 Mac.

The old code in FPU_FLD_I64 didn't do a proper conversion from a 64-bit integer to an 80-bit float.

This patch replaces the broken logic with a correct conversion. It first converts the integer to a double and then constructs a proper 80-bit long double from it, reusing the logic from FPU_ST80

What issue(s) does this PR address?

#5759

@joncampbell123
Copy link
Owner

I appreciate the fix, however please consider why the code stores to both the double and to the 80-bit long double format.

#119

In the 486 and Pentium era, before MMX, a "fast memcpy" hack was somewhat common where FILD and FSTP were used as a fast memcpy (64 bits at a time). It happens to work when the 64-bis are loaded into an 80-bit long double on x86 hardware because the mantissa is big enough to hold all the bits.

@asemarafa
Copy link
Author

Hi Jon,

Thanks for the explanation. You were right about the toontown demo in issue #119, my fix caused a regression.

I noticed dosbox staging handles this fast memcpy trick and dosent have the bug #5759 im trying to fix. They solve it with a shadow register. FILD_I64 saves the raw integer and FST_I64 checks if math happened, if not it uses the saved value for the memcpy trick otherwise it does a proper conversion.

I tried to copy this logic into my branch. Its a very simple attempt and i kept the changes in one file cause im not familiar with the codebase and didnt want to risk breaking anything else. but this approach works, it solves the fpu bug without the regression.

You can see the changes here.
master...asemarafa:dosbox-x:fix/fpu

Let me know if you would be interested in merging this instead.

@Torinde
Copy link
Contributor

Torinde commented Oct 2, 2025

@asemarafa, since you tested it and it works OK for both cases I think it'll be good if you make a PR with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants