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

Klaus Dormann's 65C02 functional tests are not passing #33

Open
KrisKennaway opened this issue May 11, 2017 · 14 comments
Open

Klaus Dormann's 65C02 functional tests are not passing #33

KrisKennaway opened this issue May 11, 2017 · 14 comments

Comments

@KrisKennaway
Copy link

KrisKennaway commented May 11, 2017

See https://github.com/KrisKennaway/py65/blob/master/py65/tests/devices/test_klaus.py for a test suite that runs these against py65.

I didn't commit the binaries because they're GPL'ed but two of them are checked in here: https://github.com/Klaus2m5/6502_65C02_functional_tests/tree/master/bin_files

The 6502_decimal_test I built as "as65 -l -m -w -h0 6502_decimal_test.a65"
Actually the rebuilt ones need a small tweak to load at 0x0a instead of 0x00 which the prebuilt ones are. Probably there's some way to make as65 do that.

The 65C02_extended_opcodes_test.bin is currently not passing. The first place it dies at is in the opcode tests for BBR/BBS/BBT which are not implemented on this simulator. Setting rkwl_wdc_op = 0 to test these opcodes as NOPs is not enough because (at least) the 5C opcode does not behave this way.

If you delete these nop_tests and the BBR/BBS/BBT ones (so that it still tests the RMB/SMB opcodes which are implemented here) then it correctly picks up the TSB/TRB bug that was fixed in #32

It now fails here

        PC  AC XR YR SP NV-BDIZC
65C02: 1585 01 0c ff fc 11111000

which is here in my version:

1576 :                  chkdad
                        ; decimal ADC / SBC zp
1576 : 08                       php             ;save carry for subtract
1577 : a50b                     lda ad1
1579 : 650c                     adc ad2         ;perform add
157b : 08                       php
157c : c50d                     cmp adrl        ;check result
                                trap_ne         ;bad result
157e : d0fe            >        bne *           ;failed not equal (non zero)

1580 : 68                       pla             ;check flags
1581 : 2983                     and #$83        ;mask N-----ZC
1583 : c50f                     cmp adrf
                                trap_ne         ;bad flags
1585 : d0fe            >        bne *           ;failed not equal (non zero)

https://github.com/Klaus2m5/6502_65C02_functional_tests/blob/master/bin_files/6502_functional_test.lst#L13099 (different address offset because I deleted code)

Adding some tracing I think it's failing when it gets to 0xb=0x99, 0xc=0x99 and carry set.

Failing test case is:

        PC  AC XR YR SP NV-BDIZC
65C02: 14ca 00 0c ff ff 01111011 $14CA: SEC  # Carry is set
        PC  AC XR YR SP NV-BDIZC
65C02: 14cb 00 0c ff ff 01111011 $14CB: JSR $1576
        PC  AC XR YR SP NV-BDIZC
65C02: 1576 00 0c ff fd 01111011 $1576: PHP
        PC  AC XR YR SP NV-BDIZC
65C02: 1577 00 0c ff fc 01111011 $1577: LDA $0b    ; #$99
        PC  AC XR YR SP NV-BDIZC
65C02: 1579 99 0c ff fc 11111001 $1579: ADC $0c   ; #$99 
        PC  AC XR YR SP NV-BDIZC
65C02: 157b 99 0c ff fc 01111001 $157B: PHP
        PC  AC XR YR SP NV-BDIZC
65C02: 157c 99 0c ff fb 01111001 $157C: CMP $0d   ; #$99
        PC  AC XR YR SP NV-BDIZC
65C02: 157e 99 0c ff fb 01111011 $157E: BNE $157e
        PC  AC XR YR SP NV-BDIZC
65C02: 1580 99 0c ff fb 01111011 $1580: PLA
        PC  AC XR YR SP NV-BDIZC
65C02: 1581 79 0c ff fc 01111001 $1581: AND #$83 # mask N-----ZC 
        PC  AC XR YR SP NV-BDIZC
65C02: 1583 01 0c ff fc 01111001 $1583: CMP $0f    ; #$81
        PC  AC XR YR SP NV-BDIZC
65C02: 1585 01 0c ff fc 11111000 $1585: BNE $1585

i.e. it's expecting N to be set which it is not.

KrisKennaway added a commit to KrisKennaway/py65 that referenced this issue May 11, 2017
trace the execution of the currently failing decimal-mode ADC test,
see mnaberez#33
@KrisKennaway
Copy link
Author

KrisKennaway commented May 11, 2017

Self-contained test case, passes on Virtual II

*0800L

0800-   38          SEC
0801-   F8          SED
0802-   A9 99       LDA   #$99
0804-   69 99       ADC   #$99
0806-   08          PHP
0807-   C9 99       CMP   #$99
0809-   D0 09       BNE   $0814
080B-   68          PLA
080C-   29 83       AND   #$83
080E-   C9 81       CMP   #$81
0810-   D0 02       BNE   $0814
0812-   D8          CLD
0813-   60          RTS
0814-   00          BRK

*0800G

*

But fails against mpu65c02:

    def test_bcd_adc_99_plus_99_with_carry(self):
        object_code = [
            0x38, 0xf8, 0xa9, 0x99, 0x69, 0x99, 0x08, 0xc9, 0x99,
            0xd0, 0x09, 0x68, 0x29, 0x83, 0xc9, 0x81, 0xd0, 0x02,
            0xd8, 0x60, 0x00
        ]
        mpu = self._make_mpu()
        self._write(mpu.memory, 0x0800, object_code)
        mpu.pc = 0x0800

        while True:
            mpu.step(trace=True)
            if mpu.memory[mpu.pc] == 0x60:
                success = True
                break
            elif mpu.memory[mpu.pc] == 0x00:
                success = False
                break
        assert success

$ pypy test_klaus.py
        PC  AC XR YR SP NV-BDIZC
65C02: 0800 00 00 00 ff 00110000 $0800: SEC
        PC  AC XR YR SP NV-BDIZC
65C02: 0801 00 00 00 ff 00110001 $0801: SED
        PC  AC XR YR SP NV-BDIZC
65C02: 0802 00 00 00 ff 00111001 $0802: LDA #$99
        PC  AC XR YR SP NV-BDIZC
65C02: 0804 99 00 00 ff 10111001 $0804: ADC #$99
        PC  AC XR YR SP NV-BDIZC
65C02: 0806 99 00 00 ff 01111001 $0806: PHP
        PC  AC XR YR SP NV-BDIZC
65C02: 0807 99 00 00 fe 01111001 $0807: CMP #$99
        PC  AC XR YR SP NV-BDIZC
65C02: 0809 99 00 00 fe 01111011 $0809: BNE $0814
        PC  AC XR YR SP NV-BDIZC
65C02: 080b 99 00 00 fe 01111011 $080B: PLA
        PC  AC XR YR SP NV-BDIZC
65C02: 080c 79 00 00 ff 01111001 $080C: AND #$83
        PC  AC XR YR SP NV-BDIZC
65C02: 080e 01 00 00 ff 01111001 $080E: CMP #$81
        PC  AC XR YR SP NV-BDIZC
65C02: 0810 01 00 00 ff 11111000 $0810: BNE $0814
F
======================================================================
FAIL: test_bcd_adc_99_plus_99_with_carry (__main__.KlausDormannTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_klaus.py", line 52, in test_bcd_adc_99_plus_99_with_carry
    assert success
AssertionError

----------------------------------------------------------------------
Ran 1 test in 0.012s

FAILED (failures=1)

@mnaberez
Copy link
Owner

See https://github.com/KrisKennaway/py65/blob/master/py65/tests/devices/test_klaus.py for a test suite that runs these against py65.

I didn't commit the binaries because they're GPL'ed but two of them are checked in here: https://github.com/Klaus2m5/6502_65C02_functional_tests/tree/master/bin_files

Yeah, that's an unfortunate license for a test suite. It can't be included in non-GPL projects like this one. It will not be acceptable to use that code here.

@KrisKennaway
Copy link
Author

We can still use it to debug though :) I think the bug is here in opADC:

            if aluresult == 0:
                self.p |= self.ZERO
            else:
                self.p |= aluresult & self.NEGATIVE

I think this should be the adjusted value, (nibble1 << 4) + nibble0
I'm also not sure about the other usage of unadjusted aluresult a few lines down that sets OVERFLOW

BTW another small issue is that this code is in mpu6502 but according to http://6502.org/tutorials/65c02opcodes.html the 6502 did not set these flags in decimal mode.

@mnaberez
Copy link
Owner

We can still use it to debug though :)

Absolutely.

@BigEd, any chance you could look at this? You had fixed some other BCD bugs in the past. I can't remember the last time I used BCD mode myself.

@KrisKennaway
Copy link
Author

I think KrisKennaway@666cd9c fixes it.

The test suite now passes (with the above caveats about unimplemented opcodes) and it claims to exhaustively test the valid BCD operations.

@BigEd
Copy link
Collaborator

BigEd commented May 12, 2017

That sounds good enough for me @KrisKennaway!

For future reference: I think Klaus' suite tests BCD last of all, so once you've got that far, it can sometimes help to swap to Bruce's decimal tests. For a 65C02 test, check out Appendix D:
http://6502.org/tutorials/decimal_mode.html#D

@BigEd
Copy link
Collaborator

BigEd commented May 12, 2017

Oh, hang on... @KrisKennaway your change is an update to mpu6502.py - I thought you were aiming to test and correct mpu65c02.py? Ideally both would be correct, with respect to the appropriate testsuite. But I'm not sure it's possible for them to be correct against the other testsuite!

@KrisKennaway
Copy link
Author

Yes you're right of course -- 6502 and 65c02 do not have mutually compatible behaviour here, neither in treatment of flags nor in behaviour for invalid operands.

I've got something that passes the full decimal mode test suite for both processors including all flags and invalid operands, I'll clean that up for submission.

Happily the relevant test code is public domain so we can also ship the source and assembled binary test code.

@mnaberez
Copy link
Owner

I've got something that passes the full decimal mode test suite for both processors including all flags and invalid operands, I'll clean that up for submission.

Excellent. Thanks for your work on this!

@johangroth
Copy link

Any chance BBR/BBS/BBT will be added to the emulator?

@mnaberez
Copy link
Owner

Any chance BBR/BBS/BBT will be added to the emulator?

Yes. The intention is that it will support all the opcodes of the WDC W65C02S.

@MorrisMA
Copy link

Just curious. The 65C02 Rockwell bit-oriented instructions are RMBx/SMBx zp, and BBRx/BBSx zp,rel8. What is the BBT instruction being discussed in a previous message?
In addition, what is the specific error that Klaus' 65C02 tests is reporting when mpu65C02.py is the selected device?

@MorrisMA
Copy link

Never mind. I've added these 16 instructions to my M65C02A Python device model, but I've not successfully gotten the built-in Py65 assembler/disassembler to recognize the syntax and produce a correct output.

Johan: if you'd like to try to incorporate the instructions into the mpu65C02.py model and get the assembler and disassembler to recognize the instructions correctly, you can look in https://github.com/MorrisMA/py65/blob/master/py65/devices/mpuM65C02A.py to see how I implemented these instructions.

I use a slightly different organization of the instruction decoder that used by the other Py65 processor device models. Instead of being organized by rows in the opcode table, my model's instruction table is organized the columns. This is done in order to match the M65C02A model to the instruction decoder incorporated into the microprogrammed Verilog implementation. Please be aware that I've not been working on this model for a while, AND I've definitely not run the model against Klaus' test suite. That statement applies to both the Python model and the Verilog model. Previous incarnations / releases of the Verilog model successfully passed Klaus' test suite with minor tweaks related to the addresses on the stack for JSR, BRK, and interrupts; these differences between my 6502/65C02 models and the standard models are clearly discussed in the accompanying documentation. Furthermore, I'm not currently including the WDC WAI and STP instructions; I am using those two opcodes for something else in my current models.

@BigEd
Copy link
Collaborator

BigEd commented Mar 7, 2019

I've got something that passes the full decimal mode test suite for both processors including all flags and invalid operands, I'll clean that up for submission.

Happily the relevant test code is public domain so we can also ship the source and assembled binary test code.

Hi @KrisKennaway it looks like you did the work to fix BCD - assuming we don't already have it, please could you send a pull request?

(I think I'd prefer to fix and close BCD with this issue, and have a new issue for missing opcodes, if indeed they are still missing.)

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

5 participants