search

atomheartother / chip8

A simple chip-8 emulator in C++ using SDL2 with desktop & browser support
https://chip8.navennec.net/
10 stars 0 forks source link

Flags of subtraction opcodes have wrong carry condition #1

Closed gulrak closed 3 weeks ago

gulrak commented 3 weeks ago

Just saw this on Reddit, quite some different approaches, nice!

I just wanted to suggest running the flags test of https://github.com/Timendus/chip8-test-suite as your carry flag for the two subtraction opcodes seems to suffer from the common mistake that subtracting equal operants needs no borrow.

atomheartother commented 3 weeks ago

image

Seems you're right, I had already run a bunch of test suites but I had not run this one. Let me see if I can squash this bug.

gulrak commented 3 weeks ago

Typically it's just to change the comparison from > to >= for the condition to set vf to 1.

gulrak commented 3 weeks ago

And typically, if other tests disagree with this in a way that makes it impossible to succeed on this test suite, the other tests are wrong. Timendus did a great job. Still, as all existing tests for CHIP-8 variant, no failed test doesn't mean all is good, but its safe to say if one of the tests in that suite fails, one actually has an issue, it also succeeds on the original COSMAC VIP and the on a HP-48SX running SCHIP. Sadly that can't be said for a bunch of older test programs.

atomheartother commented 3 weeks ago

I don't see where the case can be made that the documentation for 8xy5 should translate to VF = Vx >= Vy. I checked the documentation and it's not so clear. All docs for 8xy5 say, in so many words:

In the case of an underflow/borow, VF is set 0. Otherwise 1.

Doesn't this clearly translate to carry = vx > vy ? If x == y then x-y does NOT cause an underflow, so This is also the way most documentation online translates it, like cowgod's.

Edit: I was wrong and confusd, see below.

atomheartother commented 3 weeks ago

Oh wait I just got it, the actual spec is VF is set to NOT borrow. So this means it's ~(Vy > Vx) which does translate to Vx >= Vy. What a strange thing that so many people got this wrong in online docs, seems like it's a very common misinterpretation. Thanks for the flag.

atomheartother commented 3 weeks ago

Fixed in https://github.com/atomheartother/chip8/commit/25187fd4b06e0fb23f78ed88fc65998c51feea54

atomheartother commented 3 weeks ago

Thank you for raising! <3