Jit64: Attempt to fix updating stores with an immediate value

[Pokechu22]

This fixes at least one instance of https://bugs.dolphin-emu.org/issues/13144 (the lis followed by stfsu). This is the difference in code generated for the stfsu:

 movaps xmm0, xmm6
 call .+114328374 (0x000003a76c9a86e5)
 mov dword ptr ss:[rbp-128], 0x7fd8bf84
 sub rsp, 0x0000000000000020
 mov ecx, eax
 mov edx, 0x7fdf8118
 mov rax, 0x00007ff77ac7ba00 ; Dolphin.exe!PowerPC::Write_U32(unsigned int, unsigned int)
 call rax
 add rsp, 0x0000000000000020
+mov r14d, 0x7fe00000
 test dword ptr ss:[rbp+96], 0x00000008
 jnz .+35492252 (0x000003a767e795b4) ; exception check, I think
 mov r14d, 0x7fdf8118
 movaps dqword ptr ss:[rbp+128], xmm6

Unfortunately, this doesn't seem to fix the equivalent situation for stwu (dolphin still crashes in that case). I also don't see equivalent code I can change for loads (I think, but haven't verified, that loads are affected by the same issue).

[JosJuice]

This seems more like a workaround than a proper fix. Would keeping it as RCMode::Write work if you put the MemoryExceptionCheck call first? I'm assuming that the problem is that setting this as RCMode::Write causes the register allocation to think that the old value (the immediate value) is no longer valid.

[JosJuice]

Ah, but wait... Jit64 isn't like JitArm64. It doesn't emit the code for flushing registers and exiting as soon as you call MemoryExceptionCheck, it does it later. I suppose we might need something different then? (Is this what RevertableBind is for?)

[JosJuice]

After studying the code further, yes, I think RevertableBind is what we should be using here. Please try RevertableBind(a, RCMode::Write) and see if it works. If it does work, maybe we should have a RevertableUseNoImm function in addition to RevertableBind...?

More specifically, a should use RevertableBind when we have update && jo.memcheck for any loadstore. But when the RCMode for a is ReadWrite, it so happens that using Bind instead of RevertableBind works.

[Pokechu22]

Yes, that works properly (or at least RS2 does not crash with that when it did crash on master). I've changed it.

Unfortunately I don't really understand what is going on here.

[JosJuice]

RevertableBind was designed for the following situation:

Imagine we're emulating a regular load instruction. There's nothing complicated going on in the address calculation, and we don't have update the index register. We just need to load a value from memory and store it to a register. In the general case, this is relatively straightforward. We allocate a register using gpr.Bind(d, RCMode::Write), we load into that register, and that's it. (Of course, there's also a bunch of stuff going on to handle MMIO and such, but that's outside the scope of what we're discussing here.)

Now, let's imagine that jo.memcheck is set. This means that the load can either succeed, as before, or fail, in which case we need to bail out while keeping the state of every guest register the same as it was before. This is currently implemented in Jit64 as follows.

First, instead of using gpr.Bind(d, RCMode::Write), we use gpr.RevertableBind(d, RCMode::Write):

dolphin/Source/Core/Core/PowerPC/Jit64/Jit_LoadStore.cpp

Line 137 in 0a88c23

RCX64Reg Rd = jo.memcheck ? gpr.RevertableBind(d, RCMode::Write) : gpr.Bind(d, RCMode::Write);

When we do this, the register cache stores the current value of the guest register to ppcState (unless it already was in ppcState), ensures that a host register is allocated for the guest register, and marks the guest register as "revertable":

dolphin/Source/Core/Core/PowerPC/Jit64/RegCache/JitRegCache.cpp

Lines 733 to 739 in d6437b7

	if (m_constraints[preg].ShouldBeRevertable())
	{
	StoreFromRegister(preg, FlushMode::MaintainState);
	do_bind();
	m_regs[preg].SetRevertable();
	return;
	}

Later on, if we are bailing out because the load failed, Jit64 will write all registers to ppcState except the register(s) that we previously marked as revertable:

dolphin/Source/Core/Core/PowerPC/Jit64/Jit.cpp

Lines 1077 to 1083 in 450ca0b

	RCForkGuard gpr_guard = gpr.Fork();
	RCForkGuard fpr_guard = fpr.Fork();
	gpr.Revert();
	fpr.Revert();
	gpr.Flush();
	fpr.Flush();

What's going on with in the problem you have is really the same as in the problem I just described, just with the index register instead of the destination register. So I think it would make sense to reuse the solution we already have for this problem. It's not a perfect solution – the codegen in your initial version of this PR is in a sense better – but I would prefer to use the semantically correct solution here instead of using ReadWrite. We can always improve the "revertable" system later, and then that improvement will apply both to the destination register and the index register.