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

Fix FScale'ing zero that should return zero #4085

Merged
merged 1 commit into from
Sep 27, 2024
Merged

Conversation

pmatos
Copy link
Collaborator

@pmatos pmatos commented Sep 26, 2024

Added tests will fail without the current patch.

@pmatos
Copy link
Collaborator Author

pmatos commented Sep 26, 2024

This pcem issue sarah-walker-pcem/pcem@2e037b5#diff-56c35b563dbfc08142a6ac0ee8bd3c626b5556065b41f39732fdb3625982064cR677 made me look into this in FEX.

The table in https://www.felixcloutier.com/x86/fscale clearly shows that fscale should return zero unless st0 is either NaN or +inf.

Screenshot From 2024-09-26 13-20-17

Copy link
Member

@Sonicadvance1 Sonicadvance1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

@Sonicadvance1
Copy link
Member

Had a thought, does this return the correct sign of zero?

@pmatos
Copy link
Collaborator Author

pmatos commented Sep 26, 2024

Had a thought, does this return the correct sign of zero?

Good point! It doesn't. I assumed it didn't matter. I am still unsure how it deals with the exceptional cases of st1 being +inf or Nan, which return NaN, or when st1 is -inf, in which case it returns -/+ 0. There's an odd asymmetry here. Unfortunately, I cannot run MDK2 yet.

@pmatos
Copy link
Collaborator Author

pmatos commented Sep 26, 2024

Had a thought, does this return the correct sign of zero?

It does now! :)
Updated code and tests.

@pmatos
Copy link
Collaborator Author

pmatos commented Sep 26, 2024

Had a thought, does this return the correct sign of zero?

It does now! :) Updated code and tests.

ugh - clearly something is not working 100%.

@pmatos
Copy link
Collaborator Author

pmatos commented Sep 26, 2024

I am slightly dumbfounded that after much searching, the crashes were due to an include of softfloat_types.h into SoftFloat.h. To make matters worse, my editor keeps wanting to add it to the changeset without letting me know. Really need to understand which feature this is and disable it.

Added tests will fail without the current patch.
This will properly check if we return the correct sign of zero.
@lioncash lioncash merged commit 4d62e75 into FEX-Emu:main Sep 27, 2024
12 checks passed
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