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

Another Carry flag bug, possibly #45

Open
omarandlorraine opened this issue Aug 5, 2022 · 3 comments
Open

Another Carry flag bug, possibly #45

omarandlorraine opened this issue Aug 5, 2022 · 3 comments

Comments

@omarandlorraine
Copy link
Collaborator

omarandlorraine commented Aug 5, 2022

Hi,

This is about a state in which mre/mos6502 will compute the carry flag differently from Visual 6502.

Here's the program:

    lda #$0e
    sed
    clc
    adc #$a4

As you can see, this program exercises the decimal mode, but uses invalid data in the accumulator and the operand. Visual6502 sets the carry flag after the adc instruction here, but mre/mos6502 doesn't. Visual6502 clears the carry here.

I appreciate that this is a subtle matter and arguably is undocumented or unspecified behavior. Earlier I said that if mre/mos6502 disagrees with Visual6502, then Visual6502 is correct. But I'm not sure that's right. It depends on which model of the 6502 we're emulating here. As I recall, this kind of subtlety varies between NMOS specimens and CMOS specimens.

What do we think? Have I found a bug?

@mre
Copy link
Owner

mre commented Aug 7, 2022

Isn't the CMOS variant the one that "fixes all the bugs"? 😅
At least that's my impression when I read through https://www.liquisearch.com/mos_technology_6502/bugs_and_quirks

According to this, Visual6502 emulates the original NMOS variant.

Maybe we should try your program on more emulators or even more generally run a full emulator benchmark. There are a few demo roms out there which we could try.

I think the worst thing we could do is create a weird mix between the two, so let's do a compatibility check to see where we're at.

Would be super cool to have a feature flag to switch between the two variants. It could also act as living documentation for the differences between the two.

@omarandlorraine
Copy link
Collaborator Author

Isn't the CMOS variant the one that "fixes all the bugs"?

You are right. The CMOS chips "fix all the bugs", but they also introduce new opcodes that the earlier chips don't have. They are "bra", "phx", "phy", "plx", "ply", "stz", "trb", "tsb". There are also later variants with more instructions beyond this list. But, as far as I can see, none of this is emulated by the mos6502 crate. Therefore, we could suppose that mos6502 emulates something like what's in the Commodore 64 or Atari 2600. But with no illegal instructions.

Maybe we should [...] even more generally run a full emulator benchmark

How do you feel about fuzz testing it against perfect6502? And maybe a few others like MyLittle6502 and whatever? We will need to figure out how to call C code to do this. But only during testing of course. None of that needs to leak into a release build. But we'll discover more bugs in this crate and possibly in others as well.

Would be super cool to have a feature flag to switch between the two variants

Good idea! One thing we can do already is to disable the decimal mode for ADC and SBC instructions. That's what NES emulators need.

@mre
Copy link
Owner

mre commented Aug 7, 2022

Agree to everything you said. Let's see if we can get the fuzzing set up.
Happy to accept a PR for disabling decimal mode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants