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: Micro-optimizations in fres routine #11956

Merged
merged 2 commits into from Jun 17, 2023

Conversation

JosJuice
Copy link
Member

No description provided.

Not sure why I didn't do this to begin with. Maybe I was under the
impression that the most significant bit of a 12-bit immediate was a
sign bit.
We want to have a low number of instructions between the LDP and the
MADD so that the MADD can start immediately after the LDP finishes
even if we're on a lower-end in-order CPU.
UBFX(ARM64Reg::X1, ARM64Reg::X1, 37, 10); // Grab lower part of mantissa
LDP(IndexType::Signed, ARM64Reg::W2, ARM64Reg::W3, ARM64Reg::X2, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm unfamiliar with the exact ARM in-order semantics, but can the MOVI2R(ARM64Reg::W4, 1) here be moved further up? Would that change anything?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we could move it as far up as we want to as long as we don't move it before an instruction that uses W4/X4 (which there are none of here). But there's no scheduling benefit to doing so. This MOV can run in parallel with the LDP, assuming the CPU is capable of running instructions in parallel at all, which we kind of have to assume when deciding on scheduling since otherwise it doesn't matter how we schedule instructions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha, thanks for the explanation.

@AdmiralCurtiss
Copy link
Contributor

I assume you tested this. Reasonable to me.

@AdmiralCurtiss AdmiralCurtiss merged commit 089d433 into dolphin-emu:master Jun 17, 2023
14 checks passed
@JosJuice
Copy link
Member Author

Even better: This code has unit tests ;)

@JosJuice JosJuice deleted the jitarm64-12-bit-asm branch June 17, 2023 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants