Fix various issues with the twi/tw instructions #1017
Conversation
|
Yes, I know that PSO uses one of them in its movie player. |
|
Huh. And it doesn't pop up a dialog saying "WriteNormalOp - a1 cannot be imm"? Or is constant propagation not in effect for PSO's tw instructions? |
|
Doesn't seem to hit it with constant propagation. |
|
This is what KillImmediate/BindToRegister is for, isn't it? |
|
Which one of the two should I use? Twi just needs to compare a register against an immediate. I guess I need to pass doLoad=true to BindToRegister to have it load the value into a register, but what should I pass in for makeDirty? |
|
If the register isn't modified, set makeDirty to false. Otherwise, let it default to true. |
zhuowei
force-pushed
the
tw_fallback_if_constant_ra
branch
from
1bf8eb7
to
e31e126
Compare
|
Modified to bind constants to a register before emitting the cmp instruction. |
| @@ -1997,6 +1997,9 @@ void Jit64::twx(UGeckoInstruction inst) | |||
|
|
|||
| s32 a = inst.RA; | |||
|
|
|||
| if (gpr.R(a).IsImm()) | |||
| gpr.BindToRegister(a, true); | |||
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
if you're doing "if immediate, then bind", I think KillImmediate is what you want. if you just bind unconditionally, then use BindToRegister. |
|
Also, possibly change the PR title since it isn't falling back to Interpreter. |
zhuowei
changed the title
|
Changed the title, thanks. |
… avoid "a1 cannot be immediate" errors from the emitter
zhuowei
force-pushed
the
tw_fallback_if_constant_ra
branch
from
e31e126
to
b70c0ae
Compare
|
I switched to KillImmediate and marked makeDirty as false, since it only uses it to do one comparison. |
zhuowei
changed the title
|
I think I'm still not doing this right: Wii Linux boots farther if I disable tw/twi in the jit and make them fall back. Please don't merge this for now. |
|
Updated PR (it turn out twi was being JIT translated as a tw and vise versa) Can someone test this with a known working game that actually uses these instructions (for example, PSO)? I can only test with Wii Linux. |
zhuowei
changed the title
… the ERROR_LOG printed when tw is ran in the interpreter to DEBUG
zhuowei
force-pushed
the
tw_fallback_if_constant_ra
branch
from
3dcc41b
to
e63f7c0
Compare
|
@dolphin-emu-bot rebuild |
|
PSO still works here, but it also worked before with it being wrong. |
Fix various issues with the twi/tw instructions
| CMP(32, gpr.R(a), Imm32((s32)(s16)inst.SIMM_16)); | ||
| else // tw | ||
| CMP(32, gpr.R(a), gpr.R(inst.RB)); |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
gpr.KillImmediate(a, true, false); should be gpr.BindToRegister(a, true, false); |
|
Have opened PR #1153 to fix the PanicAlert. |
This patch fixes three issues with tw/twi emulation:
1: force propagated constant into register to avoid "a1 cannot be immediate" errors
Tw/twi is a "trap if comparison is true" instruction, which means that a cmp instruction is generated.
The emitter wants a register in the first argument to cmp, but constant propagation may try to pass in a constant instead.
For now, fall back to interpreter instead of displaying error: a proper way to translate this would be to load constant into a scratch register first like cmpXX does.
Do games actually use these two instructions? I ran into this error when attempting to run Wii Linux.
2: tw/twi instruction are switched in Jit64 and JitArm
the twi instruction was generating the comparison instruction for tw, and vise versa. This corrects that mistake.
3: fix another potential logging spam in the interpreter when tw is run
#1011 got rid of the ERROR_LOG printed when twi is run in interpreter, but I missed the same logging that was present in tw. This downgrades that log to DEBUG_LOG also.