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

docs/DSP: Add sections on 16-bit and 40-bit modes and on main and extended opcode writing to the same register #10692

Merged

Conversation

Pokechu22
Copy link
Contributor

Along with various typo fixes in both the DSP manual and comments in Dolphin's DSP code. The new sections are based on studying Dolphin's DSP interpreter, and not by hardware testing.

This is a draft because I am considering replacing the existing "Perform an additional operation depending on destination register." messages in various instructions with a more explicit back-reference to the "16-bit and 40-bit modes" subsection (along with adding it to any instructions where it's missing but exists in Dolphin), but haven't done so yet.

Copy link
Member

@BhaaLseN BhaaLseN left a comment

Choose a reason for hiding this comment

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

Seems good for the most part. I'm trying to remember why those opcodes use r instead of d, but so far I'm coming up empty 🤷

@Pokechu22
Copy link
Contributor Author

Note that there used to be a lot more inconsistencies with the opcode bits (see #10048, particularly 446b1d2). d versus r is less important; "5.3 Bit meanings" says d is destination register while r is register (either source or destination), and since ADD adds to the value already in the register, r could theoretically work, but all of the other ADD instructions use d.

@Pokechu22 Pokechu22 force-pushed the dsp-manual-set40-and-write-backlog branch from b45e73a to 4a73cdf Compare May 23, 2022 18:20
@Pokechu22 Pokechu22 force-pushed the dsp-manual-set40-and-write-backlog branch 2 times, most recently from 8c079c4 to d018f3b Compare May 25, 2022 18:21
lioncash added a commit to lioncash/dolphin that referenced this pull request May 25, 2022
Spawned off discussion in dolphin-emu#10692, this introduces a general document
that can be used to document intentional divergences from hardware
behavior (e.g. due to it being negligible, more practical, intensive to
implement properly for most games, etc).

For the initial introduction of the document, this only condenses the
information from the originating discussion.

Any other noteworthy differences can be later added by others.
lioncash added a commit to lioncash/dolphin that referenced this pull request May 25, 2022
Spawned off discussion in dolphin-emu#10692, this introduces a general document
that can be used to document intentional divergences from hardware
behavior (e.g. due to it being negligible, more practical, intensive to
implement properly for most games, etc).

For the initial introduction of the document, this only condenses the
information from the originating discussion.

Any other noteworthy differences can be later added by others.
$acS.h was a typo, which has been replaced with $acD.h.
…rite to the same register (the write backlog)

For more information, ApplyWriteBackLog, WriteToBackLog, and ZeroWriteBackLog were added in b787f5f and the explanatory comment was added in fd40513, although it did not mention the specific instructions that could trigger this edge case. The statements about which registers can be written by main opcodes and extension opcodes are based on my own checking of all instructions in the manual.
…mediate

These instructions used an 'r' in their bit list, but a 'd' in the operands.
We don't have anything called $amD, though we do have $acsD.  However, these instructions affect flags based on the whole accumulator, so it's better to just use $acD.
@Pokechu22 Pokechu22 force-pushed the dsp-manual-set40-and-write-backlog branch from d018f3b to c1a51c5 Compare May 31, 2022 23:36
@Pokechu22
Copy link
Contributor Author

I've added hardware tests for the 40-bit mode (one testing a large range of values, though not all of them as that would take several days to run, and one testing various instructions), replaced the cryptic "Perform an additional operation depending on destination register" messages with a reference to the new 40-bit mode section, and also fixed a few more typos. The new version of the manual is here: GameCube_DSP_Users_Manual.pdf.

@Pokechu22 Pokechu22 force-pushed the dsp-manual-set40-and-write-backlog branch from c1a51c5 to f47dfc3 Compare June 2, 2022 05:27
@Pokechu22
Copy link
Contributor Author

I discovered that LOOP and BLOOP also experience saturation; this has been implemented in #10716. I also realized that I didn't include 'LS in the test or the list of instructions that experience saturation; that has been fixed (and I also added a note that explains how 'LS and 'SL differ because it's not obvious at first glance).

@lioncash lioncash merged commit f7f47d3 into dolphin-emu:master Jun 3, 2022
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants