Permalink
Browse files

interpreter: fix a subtle bug in a QFSRV

Math is correct but a shift of 64 bits is illegal in x86 because the cl register is masked

From the x86 spec:
    The destination operand can be a register or a memory location. The count operand can be an immediate value or
    the CL register. The count is masked to 5 bits (or 6 bits if in 64-bit mode and REX.W is used). The count range is
    limited to 0 to 31 (or 63 if 64-bit mode and REX.W is used). A special opcode encoding is provided for a count of 1.
  • Loading branch information...
1 parent 22de865 commit 46a2f6ed24067dfdc2002ff9a1a365d941ff7779 @gregory38 gregory38 committed Jan 12, 2016
Showing with 6 additions and 2 deletions.
  1. +6 −2 pcsx2/MMI.cpp
View
@@ -1032,8 +1032,12 @@ void QFSRV() { // JayteeMaster: changed a bit to avoid screw up
*/
Rd.UD[0] = cpuRegs.GPR.r[_Rt_].UD[1] >> (sa_amt - 64);
Rd.UD[1] = cpuRegs.GPR.r[_Rs_].UD[0] >> (sa_amt - 64);
- Rd.UD[0]|= cpuRegs.GPR.r[_Rs_].UD[0] << (128 - sa_amt);
- Rd.UD[1]|= cpuRegs.GPR.r[_Rs_].UD[1] << (128 - sa_amt);
+ if (sa_amt != 64) {
+ // A 64 bit shift is equivalent to a 0 bit shift because value is masked
+ // on 6 bits
+ Rd.UD[0]|= cpuRegs.GPR.r[_Rs_].UD[0] << (128u - sa_amt);
+ Rd.UD[1]|= cpuRegs.GPR.r[_Rs_].UD[1] << (128u - sa_amt);
+ }
cpuRegs.GPR.r[_Rd_] = Rd;
}
}

4 comments on commit 46a2f6e

@gregory38
Contributor

Note: it greatly improves the rendering of Grandia3 FMV.

@bositman
Member

Should be noted this is in interpreters only, so people don't expect magic to happen. Good job nailing it BTW :)

@refractionpcsx2
Member

I expected nothing, Grandia 3 was fine anyway :P obviously on the recompilers though

@gregory38
Contributor

The future is the interpreter. I'm sure that with a couple of shortcut we can improve the perf by 20%-50% 😛

ps2autotest was really helpful on this one.

Please sign in to comment.