Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEXCore: Override x87 precision control when necessary #4194

Merged
merged 3 commits into from
Dec 10, 2024

Conversation

Sonicadvance1
Copy link
Member

According to the documentation for x87 FCW precision control, this only
affects fadd*, fsub*, fmul*, fdiv*, and fsqrt. FEX was incorrectly
reducing precision for all x87 operations.

Precision is ignored for the following x87 ALU operations:

  • fabs
  • fscale
  • fprem{1,}
  • fcos
  • fsin
  • ftan
  • fyl2x
  • fyl2xp1
  • fpatan
  • fsincos
  • Plus any operations just doing data movement and conversions

Next commit adds unittests to ensure this is correct for each
instruction.

Only missing unittests are fpatan and fsincos.

According to the documentation for x87 FCW precision control, this only
affects fadd*, fsub*, fmul*, fdiv*, and fsqrt. FEX was incorrectly
reducing precision for all x87 operations.

Precision is ignored for the following x87 ALU operations:
- fabs
- fscale
- fprem{1,}
- fcos
- fsin
- ftan
- fyl2x
- fyl2xp1
- fpatan
- fsincos
- Plus any operations just doing data movement and conversions

Next commit adds unittests to ensure this is correct for each
instruction.
Tests all the instructions that are affected by FCW PC (or not!)
Only missing tests are fsincos (More easily tested with just fsin and
fcos), and fpatan
@pmatos
Copy link
Collaborator

pmatos commented Dec 5, 2024

Nice - tests and fix! :) That's great.
LGTM.

@Sonicadvance1
Copy link
Member Author

ping for review

@lioncash lioncash merged commit 8111b7c into FEX-Emu:main Dec 10, 2024
12 checks passed
@Sonicadvance1 Sonicadvance1 deleted the fcw_pc_instructions branch December 10, 2024 22:26
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