search

RIP-Comm / clementine

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

Thumb: Implement LSL for shift #154

Closed guerinoni closed 1 year ago

guerinoni commented 1 year ago
guerinoni commented 1 year ago

Since we moved the shift logic to a new function, we lose the fact that before the code was modifying the carry variable and later changing it in the cpu. Now the shift function doesn't change the carry, thus the shift_operand function doesn't updates the carry bit in the cpsr, since it only uses the result field of the returned value from shift

I'm not sure about using ArithmeticOpResult here since it contains fields we would never use in such a context. The idea behind this struct was to carry the "new state" of an ALU operation (add/sub). Maybe we can rethink it? Maybe we could make it more "modular"

Ok now I re-designed a little bit and seems correct to me! Obviously the refactor will be nicer when all operation will be split as I did for shift... But meanwhile... :)

codecov-commenter commented 1 year ago

Codecov Report

Base: 56.26% // Head: 56.32% // Increases project coverage by +0.05% :tada:

Coverage data is based on head (92ce57e) compared to base (d94b038). Patch coverage: 43.47% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #154 +/- ## ========================================== + Coverage 56.26% 56.32% +0.05% ========================================== Files 32 32 Lines 1946 2008 +62 ========================================== + Hits 1095 1131 +36 - Misses 851 877 +26 ``` | [Impacted Files](https://codecov.io/gh/RIP-Comm/clementine/pull/154?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [emu/src/cpu/psr.rs](https://codecov.io/gh/RIP-Comm/clementine/pull/154/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-ZW11L3NyYy9jcHUvcHNyLnJz) | `80.68% <ø> (ø)` | | | [emu/src/cpu/data\_processing.rs](https://codecov.io/gh/RIP-Comm/clementine/pull/154/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-ZW11L3NyYy9jcHUvZGF0YV9wcm9jZXNzaW5nLnJz) | `75.96% <33.33%> (+3.61%)` | :arrow_up: | | [emu/src/cpu/alu\_instruction.rs](https://codecov.io/gh/RIP-Comm/clementine/pull/154/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-ZW11L3NyYy9jcHUvYWx1X2luc3RydWN0aW9uLnJz) | `54.38% <34.28%> (-31.98%)` | :arrow_down: | | [emu/src/cpu/arm7tdmi.rs](https://codecov.io/gh/RIP-Comm/clementine/pull/154/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-ZW11L3NyYy9jcHUvYXJtN3RkbWkucnM=) | `77.20% <55.88%> (-1.56%)` | :arrow_down: | | [emu/src/cpu/opcode.rs](https://codecov.io/gh/RIP-Comm/clementine/pull/154/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-ZW11L3NyYy9jcHUvb3Bjb2RlLnJz) | `85.71% <100.00%> (+0.46%)` | :arrow_up: | | [emu/src/cpu/instruction.rs](https://codecov.io/gh/RIP-Comm/clementine/pull/154/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-ZW11L3NyYy9jcHUvaW5zdHJ1Y3Rpb24ucnM=) | `77.64% <0.00%> (+2.35%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

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