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

Jit: Fix "skip redundant flushes" for skipped instructions #13261

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

JosJuice
Copy link
Member

@JosJuice JosJuice commented Jan 2, 2025

Normally when an instruction is skipped (for instance due to branch merging or the BLR optimization), the registers that would have been flushed at the end of that instruction will instead be flushed at the end of the next instruction, which is maybe not perfect, but usually good enough. However, since the addition of the "skip redundant flushes" logic in fd511a6, Dolphin has accidentally skipped flushing those registers, which creates unnecessary register pressure.

The first commit of this pull request restores the old behavior. I also have two extra commits that improve things further.

Normally when an instruction is skipped (for instance due to branch
merging or the BLR optimization), the registers that would have been
flushed at the end of that instruction will instead be flushed at the
end of the next instruction, which is maybe not perfect, but usually
good enough. However, since the addition of the "skip redundant flushes"
logic in fd511a6, Dolphin has accidentally skipped flushing those
registers, which creates unnecessary register pressure. This commit
restores the old behavior.
Now we no longer have to wait until we've compiled the next instruction
after a skipped instruction before we can flush registers.

This commit is easier to read using "Ignore whitespace".
This was technically wrong, but it only affected merged dcbt+dcbst,
and I doubt any games care about it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant