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

Jit64: Turn SNaN into QNaN in HandleNaNs #11138

Merged
merged 1 commit into from Oct 15, 2022

Conversation

JosJuice
Copy link
Member

@JosJuice JosJuice commented Oct 8, 2022

Improves accuracy but isn't known to affect any games.

This turned out to be fairly convenient to implement; ORing with the PPC default NaN will quieten SNaNs and do nothing to QNaNs.

Improves accuracy but isn't known to affect any games.

This turned out to be fairly convenient to implement; ORing with the
PPC default NaN will quieten SNaNs and do nothing to QNaNs.
@AdmiralCurtiss
Copy link
Contributor

AdmiralCurtiss commented Oct 10, 2022

I don't know whether/why this is more accurate, but the non-paired code looks correct to me if your intent is to turn signaling NaNs into quiet NaNs by forcing bit 51 to 1.

The paired code I'm not sure I fully understand.

    avx_op(&XEmitter::VCMPPD, &XEmitter::CMPPD, clobber, R(xmm), R(xmm), CMP_UNORD);
    ANDPD(clobber, MConst(psGeneratedQNaN));
    ORPD(xmm, R(clobber));

If I pseudocode this out, this essentially is:

if (isnan(xmm)) {
  clobber = /* all bits 1 */;
} else {
  clobber = /* all bits 0 */;
}
clobber &= /* quiet nan */;
xmm |= clobber;

Or in other words,

if (isnan(xmm)) {
  xmm |= /* quiet nan */;
} else {
  // no change to xmm
}

Ah, okay, yes that actually makes sense!

Copy link
Contributor

@AdmiralCurtiss AdmiralCurtiss left a comment

Choose a reason for hiding this comment

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

Code-wise approval, dunno if this is actually more accurate or not.

@JMC47
Copy link
Contributor

JMC47 commented Oct 10, 2022

have we checked to see if this fixes desyncs in 1080 Avalanche and friends with real console replays?

@JosJuice
Copy link
Member Author

@AdmiralCurtiss dolphin-emu/hwtests#54 showed that this is what a real Wii does. Supposedly this behavior is specified in IEEE 754, but I can't find anywhere where it actually says that.

@JMC47 No. I believe you tested AccurateNaNs before and it didn't seem to have any impact, in which case it's unlikely that this PR would have any impact (with AccurateNaNs enabled). But feel free to test it if you want to.

@AdmiralCurtiss AdmiralCurtiss merged commit cd1f89a into dolphin-emu:master Oct 15, 2022
11 checks passed
@JosJuice JosJuice deleted the jit64-quiet-nans branch October 15, 2022 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants