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

JitArm64: FIFO optimization improvements #10046

Merged
merged 2 commits into from Oct 22, 2022

Conversation

JosJuice
Copy link
Member

JitArm64 port of PR #4123.

@JosJuice JosJuice force-pushed the jitarm64-improve-const-stores branch from e1046a4 to 1550689 Compare August 20, 2021 13:25
@theofficialgman
Copy link

I pulled this PR and rebased on master upstream
Doesn't seem to lead to any measurable performance improvements in game when tested on TX1 (already cpu thread limited), but it also doesn't bring any regressions so thats good

@JosJuice JosJuice force-pushed the jitarm64-improve-const-stores branch 3 times, most recently from 0129a58 to 4f4ba0d Compare November 21, 2021 09:32
@dvessel
Copy link
Contributor

dvessel commented Apr 25, 2022

Hope this isn’t annoying. Going through old PR’s with these numbers. Figured out a jank way to measure frame times while I do other things.

No real changes here. Apple M1, f-zero intro and a few seconds of game play.

builds samples min max median stddev
master first run 14361.000 1.258 906.872 7.038 9.610
master second run crashed ??? ??? ??? ???
master third run 14361.000 1.203 878.735 6.914 9.367
master fourth run 14361.000 1.237 891.097 6.989 9.512
10046 first run 14361.000 1.213 908.541 6.998 9.603
10046 second run 14361.000 1.020 886.503 6.840 9.448
10046 third run 14361.000 1.121 917.706 6.852 9.653
10046 fourth run 14361.000 1.177 891.943 6.822 9.440

@JosJuice JosJuice force-pushed the jitarm64-improve-const-stores branch from 4f4ba0d to 6ff98fc Compare October 15, 2022 10:48
@JosJuice JosJuice force-pushed the jitarm64-improve-const-stores branch from 6ff98fc to 77540f2 Compare October 15, 2022 10:57
@@ -806,6 +850,12 @@ bool JitArm64::DoJit(u32 em_address, JitBlock* b, u32 nextPC)
gpr.Start(js.gpa);
fpr.Start(js.fpa);

if (js.noSpeculativeConstantsAddresses.find(js.blockStart) ==
js.noSpeculativeConstantsAddresses.end())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C++20 has unordered_set::contains which might be easier to read (though it probably has no performance difference). (The same applies to the x64 JIT.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd prefer to do it like this for now and then move them over all at once. (This isn't the first place in JitArm64 where we're using this idiom.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine. (I also noticed that the x64 JIT uses camelCase in its IntializeSpeculativeConstants, which is something that can be fixed elsewhere).

Maybe "tail call" isn't quite the right term for what this code
is doing, since it's jumping to the dispatcher rather than
returning, but it's the same optimization as for a tail call.
@JosJuice JosJuice force-pushed the jitarm64-improve-const-stores branch from 77540f2 to 351d095 Compare October 15, 2022 19:45
Copy link
Contributor

@Pokechu22 Pokechu22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Untested, but looks good to me as a port of #4123.

@JMC47 JMC47 merged commit 2153276 into dolphin-emu:master Oct 22, 2022
11 checks passed
@JosJuice JosJuice deleted the jitarm64-improve-const-stores branch October 22, 2022 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants