EE:Rec: Fix recRAMCopy block compression

[bmdhacks]

Description of Changes

Change recRAMCopy to copy blocks to the full space of the ram copy buffer, not only compressed in the first quarter of it.

Rationale behind Changes

Once upon a time recRAMCopy was an array of words and stored a copy of compiled blocks to verify if a new block overwrote the old blocks by straddling them. However it was changed to an array of u8 and the /4 was never fixed.
It still kinda works, copying blocks into a quarter-sized compressed space, it just thinks that blocks overlap way more than they do, causing recompilation storms in some situations.

Suggested Testing Steps

Play games. I haven't reproduced any recompilation storms in x86 so I can't give you a game to verify it on, but this patch is still correct even if we can't find the issue in your architecture. FFX may have an issue in the tech tree selector thingy.

Did you use AI to help find, test, or implement this issue or feature?

Yes, I'm a contributor for a fork of PCSX2 that's being developed for the ARM architecture. We use AI extensively for our development. I've got a pretty good process now where after I fix stuff in our build I go type the code in myself for PCSX2, so you get to benefit from all my misspellings and shitty prose like this. Hi! Hope you're having an awesome day.

[bmdhacks]

Just to be clear, this has no LLM generated code I wrote it all myself. All three lines of it :-)

[refractionpcsx2]

So was your extensive declaration of AI usage just a flex? If it's not relevant to this PR, there's no need to say it. if you used AI, say what you used it for, and leave it at that.

[bmdhacks]

Sorry, I was told on Discord that I would be banned if I referred to any AI code, so I just wanted to be completely above board with you all because honestly you guys scare me. I want to contribute to your project and I'm just making sure you know I'm following the rules.

[refractionpcsx2]

If you want to be completely above board, you should explain exactly what you used it for and what you did yourself. As long as the code isn't completely AI generated, you know what it's done and why, then it's not usually a huge issue.

[bmdhacks]

Cool thanks for the clarification.

[TellowKrinkle]

Once upon a time recRAMCopy was an array of words and stored a copy of compiled blocks to verify if a new block overwrote the old blocks by straddling them. However it was changed to an array of u8 and the /4 was never fixed.

Would you mind linking to the commit this is fixing?

[bmdhacks]

Once upon a time recRAMCopy was an array of words and stored a copy of compiled blocks to verify if a new block overwrote the old blocks by straddling them. However it was changed to an array of u8 and the /4 was never fixed.

Would you mind linking to the commit this is fixing?

Here's the original commit that set this section up. You can see that recRAMCopy is u32:
https://github.com/PCSX2/pcsx2/blob/3aedd8acb2f9a376d75764b2ea9eddfcb385bd49/pcsx2/x86/ix86-32/iR5900-32.cpp

Here's the change from u32 to u8 that doesn't modify the /4:
d6de2e3

This commit doesn't really change behavior but it's worth looking at for how we got here since it hoses SpatialArrayReserve:
d6de2e3

[bmdhacks]

Also I just wanted to add that despite me not finding any specific cases where this results in an x86 recompile storm, it does seem to improve EE efficiency. But I suspect that the recompile storm is out there (we had one on ARM64) and this change resulted in 8x framerate improvements in that case.

[refractionpcsx2]

It could reduce clears, that's possible. 8x seems unlikely, unless the target is a serious potato.

It was certainly overlooked when it was changed from u32 to u8, anyway.

That said, the clamp on the compare when the delay slot is at the end, do not like. It should wrap around, you can use 2 compares if need be.

[bmdhacks]

That said, the clamp on the compare when the delay slot is at the end, do not like. It should wrap around, you can use 2 compares if need be.

Sorry can you clarify? Do you want me to craft a solution that is more robust to the final-opcode-dleay-slot issue or do you agree with removjng the clamp. Theres some checks elsewhere in the code to protect against this when compilimg the blocks so while its not a contract i think the current two line fix is good enough.

[refractionpcsx2]

Well if the problem is it's going past the end of memory, the logical solution is it will wrap around to the beginning of memory, so you should copy 32bits from there.

that said I'm curious the situation where this does do this, and why pc also isn't wrapping, or have I misunderstood the situation?

You definitely don't want to be going "Ah, I just won't compare it then"

[bmdhacks]

Well if the problem is it's going past the end of memory, the logical solution is it will wrap around to the beginning of memory, so you should copy 32bits from there.

That would be true in the general case, but currently the high range of RAM is uninstalled so we'd null-deref before we ever got to the end:

pcsx2/pcsx2/Memory.cpp

Line 412 in fa50a11

vtlb_MapHandler(null_handler, Ps2MemSize::ExposedRam, 0x10000000 - Ps2MemSize::ExposedRam);

that said I'm curious the situation where this does do this, and why pc also isn't wrapping, or have I misunderstood the situation?

You definitely don't want to be going "Ah, I just won't compare it then"

Naw, I'm not doing that. I didn't understand the whole wrapping mechanism when I wrote the clamp but HWADDR does wrap. But I don't think it can wrap off the end because of the uninstalled high mem range, so I don't think the two compares are necessary.

But also... the occurrence of a guest putting a branch in RAM's final word seems insanely unlikely to me.