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: Replace "msrBits" with "featureFlags" and use for performance monitor #11988

Merged
merged 3 commits into from
Dec 5, 2023

Conversation

JosJuice
Copy link
Member

By making the JIT cache check if the current state of MMCR0 and MMRC1 matches the state they had at the time the JIT block was compiled, we solve a correctness issue (marked in a comment as a speed hack).

Not known to affect any games.

@JosJuice
Copy link
Member Author

@JMC47 Could you help me test this? Enable JIT logging on warning level, then run Resident Evil 4 as well as Harry Potter and the Prisoner of Azkaban and check:

  • If the performance is different than in master
  • If you get the log message "Feature flags mismatch! Please report if this happens a lot." repeatedly

JitArm64 support not yet implemented.

@JosJuice
Copy link
Member Author

JMC has now done the testing. It seems like having to recompile a JIT block due to mismatching feature flags happens about 400 times when booting Harry Potter and the Prisoner of Azkaban but not in other situations, and there was no noticeable performance impact when playing either of the two games I listed. So handling mismatched feature flags by always recompiling the block should be fine.

Combined with JitArm64 support being added, I now consider this PR ready.

@JosJuice JosJuice marked this pull request as ready for review August 27, 2023 10:40
@JMC47
Copy link
Contributor

JMC47 commented Sep 2, 2023

This is tested. If no one reviews it I will eventually merge this. Please review it before that happens.

@Tilka
Copy link
Member

Tilka commented Nov 28, 2023

This means we now require 64 GiB of free disk space on Windows?

@JosJuice
Copy link
Member Author

No, it works fine with a tiny amount of free disk space, just like before. But it is true that this increases the allocation you're thinking of from 32 GiB to 64 GiB.

@JosJuice JosJuice force-pushed the jit-feature-flags branch 3 times, most recently from e3869f0 to d158d8a Compare November 29, 2023 18:07
@lioncash
Copy link
Member

Needs a rebase now that the dispatcher stuff is merged

Preparation for the next commit.
By making the JIT cache check if the current state of MMCR0 and MMRC1
matches the state they had at the time the JIT block was compiled, we
solve a correctness issue (marked in a comment as a speed hack).

Not known to affect any games.
This has the same effect in the end, but in my opinion, doing it this
way makes it more clear for the reader why we can read from ppcState at
JIT time, something that makes no sense for everything else in ppcState.
@JosJuice JosJuice marked this pull request as ready for review November 30, 2023 21:40
@lioncash lioncash merged commit a65246e into dolphin-emu:master Dec 5, 2023
11 checks passed
@JosJuice JosJuice deleted the jit-feature-flags branch December 6, 2023 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants