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

Jit64: Use farcode for exception exit in twX. #10275

Merged
merged 1 commit into from Dec 18, 2021

Conversation

AdmiralCurtiss
Copy link
Contributor

See https://forums.dolphin-emu.org/Thread-new-5-0-15445-causes-jump-target-too-far-away-needs-force5bytes-true

I didn't do it yet, but it might be worth bisecting what change exactly caused the distance to become too long?

@JosJuice
Copy link
Member

I added the following extra instruction when fixing True Crime. Maybe that managed to push it over the edge...

MOV(32, PPCSTATE_SRR1, Imm32(static_cast<u32>(ProgramExceptionCause::Trap)));

Do you think it would make sense to make twX use farcode? Or is the exception path commonly taken?

@AdmiralCurtiss
Copy link
Contributor Author

I have not even the slightest idea how often this path is taken. Would be nice if we had metrics like that... But yes, if it's somewhat rare that's probably not a bad idea.

@JosJuice
Copy link
Member

tw and twi are the only instructions that generate ProgramExceptionCause::Trap, so one way to measure how common it is could be to add an if statement here that outputs a message to the log if SRR1 == ProgramExceptionCause::Trap:

@saulfabregwiivc
Copy link

saulfabregwiivc commented Dec 15, 2021

@AdmiralCurtiss @JosJuice

I tested this build with this pull request on Windows x64 [pr-10275-win-x64] (i saw that is based in Dolphin 5.0-15605) x64 and the problem of "Jump target too far away, needs force5Bytes = true" is now gone, now the Not64 emulator works again as a charm, as i specified in my thread (https://forums.dolphin-emu.org/Thread-new-5-0-15445-causes-jump-target-too-far-away-needs-force5bytes-true) with old 5.0-15262.

Will you bring this fix for a new development or beta release?

@Sintendo
Copy link
Member

Do we know of any other titles using tw/twi? Issue 7253 mentions Monopoly, but I couldn't hit this instruction for any title that I happened to have at hand.

@JMC47
Copy link
Contributor

JMC47 commented Dec 17, 2021

Does forcing 5 bytes mean that we lose performance? Does this cost performance in general?

@JosJuice
Copy link
Member

It will make the code size a little larger. In some cases, this can cause a small performance drop due to less code being able to fit in the icache.

@AdmiralCurtiss
Copy link
Contributor Author

The difference should be minuscule in practice though, especially since apparently this instruction is barely used by games.

@AdmiralCurtiss
Copy link
Contributor Author

I hacked myself some metrics and didn't encounter a single exceptional exit while playing around, so yes moving this to farcode is probably a good idea.

As a byproduct, this fixes the dont_trap jump not having enough available jump distance to its target in some instances (eg. in Not64).
@AdmiralCurtiss AdmiralCurtiss changed the title Jit64: Use a 5 byte jump in twX, the short one seems to be not enough sometimes (eg. Not64 emulator). Jit64: Use farcode for exception exit in twX. Dec 18, 2021
@JMC47 JMC47 merged commit d8e347c into dolphin-emu:master Dec 18, 2021
9 of 10 checks passed
@AdmiralCurtiss AdmiralCurtiss deleted the twX-5-byte-jmp branch December 18, 2021 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants