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: Enforce correct alignment of SPR_TL #11074

Merged
merged 1 commit into from Sep 20, 2022

Conversation

JosJuice
Copy link
Member

@dvessel Could you check if this solves the issue you described in #10808 (comment)?

@dvessel
Copy link
Contributor

dvessel commented Sep 20, 2022

Yes it does. Thanks for the quick fix!

@Pokechu22
Copy link
Contributor

Does it make sense that a 64-bit STR is being used instead of a 32-bit one? Do we know that it won't clobber anything else? (I think it's 64-bit because PPC_REG is Arm64Gen::ARM64Reg::X29, and X29 is 64-bit (W29 is 32-bit, I think?).)

@JosJuice
Copy link
Member Author

It's using a 64-bit write to write to SPR_TL and SPR_TU at the same time. Jit64 does the same thing.

(The reason why the write is 64-bit is because Xresult is 64-bit, not because PPC_REG is 64-bit.)

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.

Ah, that seems reasonable, then.

Incidentally, I decided to check the ppc_750cl manual, which mentions (page 113/114):

The TB registers are referred to as TBRs rather than SPRs and can be written to using the mtspr instruction in supervisor mode and the TBR numbers here. The TB registers can be read in user mode using either the mftb or mfspr instruction and specifying TBR 268 for TBL and SPR 269 for TBU.

And also, page 117:

Note that the 750CL ignores the extended opcode differences between mftb and mfspr by ignoring bit 25 and treating both instructions identically.

It looks like we have mftb just call mfspr, but some of the optimisations in mfspr for TB expect it to be mftb.

It also looks like we don't handle moves to the timebase, which is probably not that important but I think I've seen it happen. The manual also mentions a mttb instruction in the table on page 239 which doesn't appear anywhere else (this PowerPC 601 manual says it exists for compatibility but just triggers an illegal instruction there (p. 263); it's probably the same case here).

@JosJuice JosJuice merged commit 4ea694a into dolphin-emu:master Sep 20, 2022
11 checks passed
@JosJuice JosJuice deleted the jitarm64-spr-tl-alignment branch September 20, 2022 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants