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

DSPInterpreter: Fix IsLess #10031

Merged
merged 1 commit into from Aug 16, 2021
Merged

Conversation

Pokechu22
Copy link
Contributor

@Pokechu22 Pokechu22 commented Aug 16, 2021

IsLess would incorrectly return true if both SR_OVERFLOW and SR_SIGN are set, as (sr & SR_OVERFLOW) != (sr & SR_SIGN) becomes SR_OVERFLOW != SR_SIGN which is true as the two masks are different. This broke in e651592.

This issue only affected the DSP LLE Interpreter, and not the DSP LLE JIT.

I've also included a simple test case for this. ax0.l (on the top left) is set to 0 if the instruction following IFL does not execute and to 1 if it is executed.

On console

Screenshot 2021-08-15 16-41-00

On Dolphin with JIT

OHBCHB_2021-08-15_17-23-54

On Dolphin before this change

OHBCHB_2021-08-15_16-50-00

On Dolphin with this change

OHBCHB_2021-08-15_16-58-44

@lioncash
Copy link
Member

The commit message itself should probably contain the PR description (difference images aren't necessary though), so that the context of what's being fixed is more obvious (and stores that info directly in the repository).

LGTM otherwise though.

`IsLess` would incorrectly return true if both `SR_OVERFLOW` and `SR_SIGN` are set, as `(sr & SR_OVERFLOW) != (sr & SR_SIGN)` becomes `SR_OVERFLOW != SR_SIGN` which is true as the two masks are different.  This broke in e651592.

This issue only affected the DSP LLE Interpreter, and not the DSP LLE JIT.

I've also included a simple test case for this.  `ax0.l` (on the top left) is set to 0 if the instruction following `IFL` does not execute and to 1 if it is executed.
@lioncash lioncash merged commit 35c64d1 into dolphin-emu:master Aug 16, 2021
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants