RIP-Comm / clementine

Gameboy Advance emulator.
MIT License
49 stars 13 forks source link

Arm: Implement mul opcodes (halfwords and non-halfwords) #196

Closed rhighs closed 1 year ago

rhighs commented 1 year ago

This PR brings the implementation of the various opcodes for multiplications in arm-mode, I decided to keep the already present ArmModeInstruction::Multiply type and extending it a little with a ArmModeMultiplyKind to allow for a basic way of differentiating the opcodes.

Overflows are checked using u64|u32::overflowing_mul(op1, op2) which conveniently returns a tuple containing the mul result and whether an overflow occurred. I chose this way as it clearly shows the intent of what's happening there, perhaps we could only use such function when the S flag is set, otherwise there's really no point in using that. (It might bring extra overhead for no reason, but you guys tell me if that's any good to keep it like so).

Other than that, I don't really have anything important here to address. The opcodes are quite similar to each other, and it's not guaranteed we're gonna need them all, but still I brought them as it required little effort to do so.

As for the testing side I tried to test the most common case for each one, some need to check for condition flags, some don't and so on...

Looking forward to improve this as much as we can! :v:

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 97.29% and project coverage change: +2.89 :tada:

Comparison is base (68cff85) 56.25% compared to head (ceadf9f) 59.14%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #196 +/- ## ========================================== + Coverage 56.25% 59.14% +2.89% ========================================== Files 40 40 Lines 2832 3030 +198 ========================================== + Hits 1593 1792 +199 + Misses 1239 1238 -1 ``` | [Impacted Files](https://app.codecov.io/gh/RIP-Comm/clementine/pull/196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [emu/src/cpu/arm/mode.rs](https://app.codecov.io/gh/RIP-Comm/clementine/pull/196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-ZW11L3NyYy9jcHUvYXJtL21vZGUucnM=) | `17.07% <0.00%> (-0.88%)` | :arrow_down: | | [emu/src/cpu/arm/instructions.rs](https://app.codecov.io/gh/RIP-Comm/clementine/pull/196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-ZW11L3NyYy9jcHUvYXJtL2luc3RydWN0aW9ucy5ycw==) | `82.82% <97.95%> (+5.11%)` | :arrow_up: | | [emu/src/cpu/arm/operations.rs](https://app.codecov.io/gh/RIP-Comm/clementine/pull/196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-ZW11L3NyYy9jcHUvYXJtL29wZXJhdGlvbnMucnM=) | `90.18% <100.00%> (+3.64%)` | :arrow_up: | | [emu/src/cpu/arm7tdmi.rs](https://app.codecov.io/gh/RIP-Comm/clementine/pull/196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-ZW11L3NyYy9jcHUvYXJtN3RkbWkucnM=) | `72.04% <100.00%> (+0.93%)` | :arrow_up: | | [emu/src/cpu/flags.rs](https://app.codecov.io/gh/RIP-Comm/clementine/pull/196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-ZW11L3NyYy9jcHUvZmxhZ3MucnM=) | `71.42% <100.00%> (+2.19%)` | :arrow_up: | | [emu/src/cpu/thumb/operations.rs](https://app.codecov.io/gh/RIP-Comm/clementine/pull/196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-ZW11L3NyYy9jcHUvdGh1bWIvb3BlcmF0aW9ucy5ycw==) | `86.56% <100.00%> (+0.27%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.