commanderx16 / x16-emulator

Emulator for the Commander X16 8-bit computer
384 stars 60 forks source link

JMP (a,x) does not like base address spanning pages #207

Closed SlithyMatt closed 4 years ago

SlithyMatt commented 4 years ago

Not sure if this is reflective of actual 65C02 behavior, but I found (by infuriating accident) that the JMP (a,x) instruction does not work as expected when the "a" in question spans two RAM pages.

For example: 16FA: LDX #0 16FC: JMP ($16FF,X) 16FF: #$03 1700: #$17 1701: #$06 1702: #$17 1703: JMP case0 1706: JMP case1

One would expect this code to jump to $1703 and then to case0. Instead, it goes to execute $1403, seemingly using some random garbage for the high byte instead of #$17 which can be read at $1700. It appears to get that high byte from $1600 instead, due to some overflow error going back to the top of the page rather than going to the next one to retrieve the rest of the target address. If X is set to 1 instead, the jump executes as expected, since the whole address is on the same page, despite the JMP instruction sitting on a different page.

Again, I don't know if this is "right" in the sense of actual hardware behavior. I found no official warning of having to do page alignment to make this work correctly. Merely inserting a NOP at $16FF and changing the previous instruction to JMP ($1700,X) fixes it. So there is a workaround, but discovering this issue wasted a lot of time as it was hard to detect without extremely careful examination.

paulscottrobson commented 4 years ago

No, this is correct. The function ainx() does indeed replicate this 6502 bug - deliberately ! Fix is in https://github.com/commanderx16/x16-emulator/blob/master/cpu/65c02.h , function ainx()

FreeFull commented 4 years ago

From what I have read, this bug exists in the original 6502 but was fixed in the 65c02, so the emulator should not be emulating it. Edit: I am not 100% certain though, since here it's (a,x) rather than just (a)

greg-king5 commented 4 years ago

From what I have read, this bug exists in the original 6502; but, was fixed in the 65c02, so the emulator should not be emulating it. Edit: I am not 100% certain though, since here it's (a,x) rather than just (a).

Yes, the cycle counts of the original and CMOS jmp (a) and the CMOS-only jmp (a,x) suggest that the page-crossing bug was fixed for (a), but then re-introduced for (a,x).

address mode cycle count
6502 (a) 5
65C02 (a) 6
65C02 (a,x) 6
PeterCamilleri commented 4 years ago

I think that this may be a missing note in the datasheet. An earlier data sheet adds this note: "Add 1 cycle for indexing across page boundaries, or write." Only one extra clock cycle would ever be needed since the worst case $10FF + $FF + $1 is $11FF, which only crosses one page boundary.

paulscottrobson commented 4 years ago

It's usually not worth bothering with cycle level accuracy unless you are coding something like an Atari VCS emulator where it has a direct effect.

PeterCamilleri commented 4 years ago

Maybe my point is not clear. The processor clearly allocates the time needed to propagate the carry from low byte to high byte. So I think that the silicon gets it right.

mist64 commented 4 years ago

I found no evidence online that the 65C02 has a bug in the ($nnnn,X) addressing mode. The emulator should be changed to do this correctly.

FreeFull commented 4 years ago

I don't have the hardware, but it would be nice if someone who does could quickly test this.

paulscottrobson commented 4 years ago

cpu/65C02.h line 36 needs to be eahelp2 = eahelp+1;

mist64 commented 4 years ago

Fixed in 89b13a6