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.

Sign up for GitHub

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

Jump to bottom

PowerPC: Raise alignment exceptions in more situations #9865

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

PowerPC: Raise alignment exceptions in more situations #9865

wants to merge 5 commits into from

Conversation

JosJuice
Copy link
Member

@JosJuice JosJuice commented Jul 5, 2021 •
edited
Loading

Intends to improve https://bugs.dolphin-emu.org/issues/12565.

To avoid affecting the performance, the JITs will most of the time not raise alignment exceptions unless you enable the new INI-only setting AlignmentExceptions.

JosJuice
JosJuice commented Jul 5, 2021
View reviewed changes
Source/Core/Core/PowerPC/Jit64Common/Jit64AsmCommon.cpp
@@ -340,6 +340,9 @@ alignas(16) static const float m_m128 = -128.0f;
// Sizes of the various quantized store types
constexpr std::array<u8, 8> sizes{{32, 0, 0, 0, 8, 16, 8, 16}};

// TODO: Use the actual instruction being emulated (needed for alignment exception emulation)
static const UGeckoInstruction ps_placeholder_instruction = 0;
Copy link
Member Author

Choose a reason for hiding this comment

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

Could someone who knows Jit64 better give me some advice here? Basically, I want to pass an additional argument to the quantized loadstore routines (so that the slowmem handler then can forward the argument when calling Read_* or Write_*), but all of RSCRATCH, RSCRATCH2 and RSCRATCH_EXTRA are already used for arguments to the quantized loadstore routines. How should I pass the argument?

@JosJuice JosJuice force-pushed the alignment-exceptions branch 3 times, most recently from ec3dcf7 to 82fbdd2 Compare July 5, 2021 10:21
@JosJuice JosJuice marked this pull request as draft July 5, 2021 13:06
@JosJuice JosJuice force-pushed the alignment-exceptions branch from 82fbdd2 to bf78d50 Compare July 5, 2021 21:05
@JosJuice JosJuice mentioned this pull request Jul 5, 2021
Merged
@JosJuice JosJuice force-pushed the alignment-exceptions branch 3 times, most recently from ce0ff5a to 4095bb4 Compare July 11, 2021 11:08
@JosJuice JosJuice force-pushed the alignment-exceptions branch 3 times, most recently from 8f6a506 to f2fe307 Compare July 27, 2021 07:49
@JosJuice JosJuice mentioned this pull request Jul 27, 2021
Merged
@JosJuice JosJuice force-pushed the alignment-exceptions branch 5 times, most recently from 009a9d5 to bc62fca Compare August 16, 2021 17:54
@JosJuice JosJuice marked this pull request as ready for review August 16, 2021 18:01
@JosJuice
Copy link
Member Author

While alignment exceptions turned out to have nothing to do with issue 12565, alignment exceptions are still something that would make sense for us to emulate (even if it doesn't affect any games). I've cleaned up the PR after the merge of the PR that fixed 12565, and it should be more or less done other than my earlier question about Jit64.

@JosJuice JosJuice force-pushed the alignment-exceptions branch 2 times, most recently from be099c2 to bc08553 Compare August 16, 2021 19:45
@JosJuice JosJuice force-pushed the alignment-exceptions branch 4 times, most recently from 8a8cbc4 to 952c071 Compare September 5, 2021 20:19
@JosJuice JosJuice force-pushed the alignment-exceptions branch from 952c071 to 2295ba3 Compare September 21, 2021 14:27
@JosJuice JosJuice force-pushed the alignment-exceptions branch 2 times, most recently from 62a4279 to ff28ef5 Compare October 13, 2021 21:07
@JosJuice JosJuice force-pushed the alignment-exceptions branch 4 times, most recently from 7e94d09 to 09fa2dc Compare September 2, 2023 17:24
@JosJuice JosJuice force-pushed the alignment-exceptions branch from 09fa2dc to 1cf28ed Compare November 4, 2023 11:25
@JosJuice JosJuice force-pushed the alignment-exceptions branch 2 times, most recently from 4ee50ff to 966dd51 Compare November 13, 2023 18:29
@JosJuice JosJuice force-pushed the alignment-exceptions branch from 966dd51 to 134cbb9 Compare November 25, 2023 17:11
@JosJuice JosJuice force-pushed the alignment-exceptions branch 3 times, most recently from 7b8ae93 to 222e159 Compare January 12, 2024 18:57
@JosJuice JosJuice force-pushed the alignment-exceptions branch from 222e159 to a15476f Compare January 21, 2024 16:28
@JosJuice JosJuice force-pushed the alignment-exceptions branch from a15476f to d547e0e Compare February 24, 2024 19:55
@JosJuice JosJuice force-pushed the alignment-exceptions branch 2 times, most recently from d92c173 to a2ba3fc Compare April 20, 2024 15:43
@JosJuice JosJuice force-pushed the alignment-exceptions branch 2 times, most recently from 0a63799 to 04ae7b7 Compare May 4, 2024 20:17
@dreamsyntax
Copy link
Member

This might be expected behavior, but this never raises as an unhandled exception for me, and instead goes to a silent PPCHalt.

@dreamsyntax dreamsyntax mentioned this pull request Jun 30, 2024
Merged
@JosJuice
Copy link
Member Author

JosJuice commented Jul 1, 2024

Does that differ from the behavior on console?

@dreamsyntax
Copy link
Member

Does that differ from the behavior on console?

Just checked, hardware throws

Unhandled Exception 5

And verified Dolphin does too.

So it is working as expected. Panic Handlers do not get invoked however, and imo maybe they should? Though that would not be related to this PR.

@JosJuice
Copy link
Member Author

JosJuice commented Jul 18, 2024 •
edited
Loading

Please note that this PR is not done. My review comment from July 2021 still needs to be resolved in some way.

@JosJuice
PowerPC: Don't overwrite argument in ReadFromHardware/WriteToHardware
3a3be20 
Cleanup, and preparation for one of the upcoming commits.
@JosJuice JosJuice force-pushed the alignment-exceptions branch from 04ae7b7 to 3acf339 Compare July 24, 2024 17:09
JosJuice added 4 commits July 24, 2024 19:15
@JosJuice
PowerPC: Use broader mask for checking loadstore exceptions
1102a8b 
We will need to check for EXCEPTION_ALIGNMENT in addition to
EXCEPTION_DSI. Let's also throw in EXCEPTION_FAKE_MEMCHECK_HIT
while we're at it so we can skip doing fake DSI exceptions for that.

Reordering the exceptions enum is done for the sake of saving one
instruction on AArch64 when checking for loadstore exceptions.
(Bitwise operations can encode immediates containing a run of ones
of an arbitrary length, rotated by an arbitrary amount of bits.
In order to make use of this, all loadstore exceptions should
have adjacent bit positions.)
@JosJuice
PowerPC: Pass current instruction to Read/Write functions
ef512e8
@JosJuice
Add AlignmentExceptions setting
1211a6b
@JosJuice
PowerPC: Raise alignment exceptions in more situations
7e9a497 
To avoid affecting performance, the JITs will most of the time not raise
alignment exceptions unless you enable the new INI-only setting
AlignmentExceptions.
@JosJuice JosJuice force-pushed the alignment-exceptions branch from 3acf339 to 7e9a497 Compare July 24, 2024 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dreamsyntax dreamsyntax dreamsyntax approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JosJuice @phire @dreamsyntax