From 7f6b8d3587876dcc1ca42ba5566cbeb398a2fa00 Mon Sep 17 00:00:00 2001 From: Peter Evans Date: Tue, 9 Jan 2018 20:59:14 -0600 Subject: [PATCH] We can no longer assume PC increments during address handling This change required a number of consequent changes to assumptions we'd made, and I'm not 100% confident we have things right at this point in time. --- src/mos6502.addr.c | 10 ++++----- src/mos6502.branch.c | 2 +- src/mos6502.exec.c | 2 +- tests/mos6502.addr.c | 48 +++++++++++++++++++----------------------- tests/mos6502.branch.c | 8 +++---- tests/mos6502.c | 26 ++++++----------------- tests/mos6502.exec.c | 5 +---- 7 files changed, 40 insertions(+), 61 deletions(-) diff --git a/src/mos6502.addr.c b/src/mos6502.addr.c index 655c72d..4cc9265 100644 --- a/src/mos6502.addr.c +++ b/src/mos6502.addr.c @@ -44,8 +44,8 @@ static int addr_modes[] = { #define ADDR_HILO(cpu) \ vm_16bit addr; \ vm_8bit hi, lo; \ - lo = mos6502_next_byte(cpu); \ - hi = mos6502_next_byte(cpu); \ + lo = vm_segment_get(cpu->memory, cpu->PC + 1); \ + hi = vm_segment_get(cpu->memory, cpu->PC + 2); \ addr = (hi << 8) | lo /* @@ -54,7 +54,7 @@ static int addr_modes[] = { */ #define ADDR_LO(cpu) \ vm_16bit addr; \ - addr = mos6502_next_byte(cpu) + addr = vm_segment_get(cpu->memory, cpu->PC + 1) /* * This will both define the `eff_addr` variable (which is the effective @@ -140,7 +140,7 @@ DEFINE_ADDR(aby) DEFINE_ADDR(imm) { EFF_ADDR(0); - return mos6502_next_byte(cpu); + return vm_segment_get(cpu->memory, cpu->PC + 1); } /* @@ -205,7 +205,7 @@ DEFINE_ADDR(rel) vm_16bit reladdr; ADDR_LO(cpu); - reladdr = cpu->PC + addr; + reladdr = cpu->PC + addr + 2; if (addr > 127) { // If the address has the 8th bit high, then we treat the diff --git a/src/mos6502.branch.c b/src/mos6502.branch.c index 80cdd3d..5e59aa4 100644 --- a/src/mos6502.branch.c +++ b/src/mos6502.branch.c @@ -14,7 +14,7 @@ * program counter to the last effective address. */ #define JUMP_IF(cond) \ - if (cond) cpu->PC = cpu->last_addr + if (cond) cpu->PC = cpu->last_addr; else cpu->PC += 2 /* * Branch if the carry flag is clear. diff --git a/src/mos6502.exec.c b/src/mos6502.exec.c index 061296b..637ce6c 100644 --- a/src/mos6502.exec.c +++ b/src/mos6502.exec.c @@ -36,7 +36,7 @@ DEFINE_INST(jmp) */ DEFINE_INST(jsr) { - mos6502_push_stack(cpu, cpu->PC); + mos6502_push_stack(cpu, cpu->PC + 3); cpu->PC = cpu->last_addr; } diff --git a/tests/mos6502.addr.c b/tests/mos6502.addr.c index 46e548b..5cf9d7d 100644 --- a/tests/mos6502.addr.c +++ b/tests/mos6502.addr.c @@ -22,16 +22,16 @@ Test(mos6502_addr, addr_mode_acc) Test(mos6502_addr, addr_mode_abs) { vm_segment_set(cpu->memory, 0x1234, 111); - SET_PC_BYTE(cpu, 0, 0x34); - SET_PC_BYTE(cpu, 1, 0x12); + SET_PC_BYTE(cpu, 1, 0x34); + SET_PC_BYTE(cpu, 2, 0x12); cr_assert_eq(mos6502_resolve_abs(cpu), 111); } Test(mos6502_addr, addr_mode_abx_carry0) { vm_segment_set(cpu->memory, 0x1234, 111); - SET_PC_BYTE(cpu, 0, 0x30); - SET_PC_BYTE(cpu, 1, 0x12); + SET_PC_BYTE(cpu, 1, 0x30); + SET_PC_BYTE(cpu, 2, 0x12); cpu->X = 4; cr_assert_eq(mos6502_resolve_abx(cpu), 111); } @@ -39,8 +39,8 @@ Test(mos6502_addr, addr_mode_abx_carry0) Test(mos6502_addr, addr_mode_abx_carry1) { vm_segment_set(cpu->memory, 0x1234, 111); - SET_PC_BYTE(cpu, 0, 0x30); - SET_PC_BYTE(cpu, 1, 0x12); + SET_PC_BYTE(cpu, 1, 0x30); + SET_PC_BYTE(cpu, 2, 0x12); cpu->X = 3; cpu->P = cpu->P | MOS_CARRY; cr_assert_eq(mos6502_resolve_abx(cpu), 111); @@ -49,8 +49,8 @@ Test(mos6502_addr, addr_mode_abx_carry1) Test(mos6502_addr, addr_mode_aby_carry0) { vm_segment_set(cpu->memory, 0x1234, 111); - SET_PC_BYTE(cpu, 0, 0x30); - SET_PC_BYTE(cpu, 1, 0x12); + SET_PC_BYTE(cpu, 1, 0x30); + SET_PC_BYTE(cpu, 2, 0x12); cpu->Y = 4; cr_assert_eq(mos6502_resolve_aby(cpu), 111); } @@ -58,8 +58,8 @@ Test(mos6502_addr, addr_mode_aby_carry0) Test(mos6502_addr, addr_mode_aby_carry1) { vm_segment_set(cpu->memory, 0x1234, 111); - SET_PC_BYTE(cpu, 0, 0x30); - SET_PC_BYTE(cpu, 1, 0x12); + SET_PC_BYTE(cpu, 1, 0x30); + SET_PC_BYTE(cpu, 2, 0x12); cpu->Y = 3; cpu->P = cpu->P | MOS_CARRY; cr_assert_eq(mos6502_resolve_aby(cpu), 111); @@ -67,7 +67,7 @@ Test(mos6502_addr, addr_mode_aby_carry1) Test(mos6502_addr, addr_mode_imm) { - SET_PC_BYTE(cpu, 0, 0x12); + SET_PC_BYTE(cpu, 1, 0x12); cr_assert_eq(mos6502_resolve_imm(cpu), 0x12); } @@ -76,7 +76,7 @@ Test(mos6502_addr, addr_mode_idx) vm_segment_set(cpu->memory, 0x17, 0x23); vm_segment_set(cpu->memory, 0x23, 123); - SET_PC_BYTE(cpu, 0, 0x12); + SET_PC_BYTE(cpu, 1, 0x12); cpu->X = 5; cr_assert_eq(mos6502_resolve_idx(cpu), 123); } @@ -86,7 +86,7 @@ Test(mos6502_addr, addr_mode_idy) vm_segment_set(cpu->memory, 0x12, 0x23); vm_segment_set(cpu->memory, 0x28, 123); - SET_PC_BYTE(cpu, 0, 0x12); + SET_PC_BYTE(cpu, 1, 0x12); cpu->Y = 5; cr_assert_eq(mos6502_resolve_idy(cpu), 123); } @@ -97,42 +97,38 @@ Test(mos6502_addr, addr_mode_ind) vm_segment_set(cpu->memory, 0x1235, 0x23); vm_segment_set(cpu->memory, 0x2345, 123); - SET_PC_BYTE(cpu, 0, 0x34); - SET_PC_BYTE(cpu, 1, 0x12); + SET_PC_BYTE(cpu, 1, 0x34); + SET_PC_BYTE(cpu, 2, 0x12); cr_assert_eq(mos6502_resolve_ind(cpu), 123); } Test(mos6502_addr, addr_mode_rel_positive) { cpu->PC = 123; - SET_PC_BYTE(cpu, 0, 88); + SET_PC_BYTE(cpu, 1, 88); cr_assert_eq(mos6502_resolve_rel(cpu), 0); - cr_assert_eq(cpu->last_addr, 212); + cr_assert_eq(cpu->last_addr, 213); } Test(mos6502_addr, addr_mode_rel_negative) { cpu->PC = 123; - SET_PC_BYTE(cpu, 0, 216); + SET_PC_BYTE(cpu, 1, 216); cr_assert_eq(mos6502_resolve_rel(cpu), 0); - - // Why not 83? Because when we resolve the relative address, PC will - // have incremented to account for the byte operand. So the correct - // calculation seems to be 123 + 1 + 216 - 256, which is 84. - cr_assert_eq(cpu->last_addr, 84); + cr_assert_eq(cpu->last_addr, 85); } Test(mos6502_addr, addr_mode_zpg) { vm_segment_set(cpu->memory, 0x0034, 222); - SET_PC_BYTE(cpu, 0, 0x34); + SET_PC_BYTE(cpu, 1, 0x34); cr_assert_eq(mos6502_resolve_zpg(cpu), 222); } Test(mos6502_addr, addr_mode_zpx) { vm_segment_set(cpu->memory, 0x0034, 222); - SET_PC_BYTE(cpu, 0, 0x30); + SET_PC_BYTE(cpu, 1, 0x30); cpu->X = 4; cr_assert_eq(mos6502_resolve_zpx(cpu), 222); } @@ -140,7 +136,7 @@ Test(mos6502_addr, addr_mode_zpx) Test(mos6502_addr, addr_mode_zpy) { vm_segment_set(cpu->memory, 0x0034, 222); - SET_PC_BYTE(cpu, 0, 0x2F); + SET_PC_BYTE(cpu, 1, 0x2F); cpu->Y = 5; cr_assert_eq(mos6502_resolve_zpy(cpu), 222); } diff --git a/tests/mos6502.branch.c b/tests/mos6502.branch.c index b97c55d..76009cc 100644 --- a/tests/mos6502.branch.c +++ b/tests/mos6502.branch.c @@ -15,7 +15,7 @@ Test(mos6502_branch, bcc) cpu->P |= MOS_CARRY; cpu->last_addr = 125; mos6502_handle_bcc(cpu, 0); - cr_assert_neq(cpu->PC, 125); + cr_assert_neq(cpu->PC, 127); } Test(mos6502_branch, bcs) @@ -63,7 +63,7 @@ Test(mos6502_branch, bne) cpu->P |= MOS_ZERO; cpu->last_addr = 125; mos6502_handle_bne(cpu, 0); - cr_assert_neq(cpu->PC, 125); + cr_assert_neq(cpu->PC, 127); } Test(mos6502_branch, bpl) @@ -75,7 +75,7 @@ Test(mos6502_branch, bpl) cpu->P |= MOS_NEGATIVE; cpu->last_addr = 125; mos6502_handle_bpl(cpu, 0); - cr_assert_neq(cpu->PC, 125); + cr_assert_neq(cpu->PC, 127); } Test(mos6502_branch, bvc) @@ -87,7 +87,7 @@ Test(mos6502_branch, bvc) cpu->P |= MOS_OVERFLOW; cpu->last_addr = 125; mos6502_handle_bvc(cpu, 0); - cr_assert_neq(cpu->PC, 125); + cr_assert_neq(cpu->PC, 127); } Test(mos6502_branch, bvs) diff --git a/tests/mos6502.c b/tests/mos6502.c index 16b4ff0..6ee76cb 100644 --- a/tests/mos6502.c +++ b/tests/mos6502.c @@ -22,18 +22,6 @@ Test(mos6502, create) cr_assert_eq(cpu->S, 0); } -Test(mos6502, next_byte) -{ - cpu->PC = 128; - vm_segment_set(cpu->memory, cpu->PC, 123); - vm_segment_set(cpu->memory, cpu->PC + 1, 234); - vm_segment_set(cpu->memory, cpu->PC + 2, 12); - - cr_assert_eq(mos6502_next_byte(cpu), 123); - cr_assert_eq(mos6502_next_byte(cpu), 234); - cr_assert_eq(mos6502_next_byte(cpu), 12); -} - Test(mos6502, push_stack) { mos6502_push_stack(cpu, 0x1234); @@ -109,17 +97,13 @@ Test(mos6502, get_instruction_handler) Test(mos6502, execute) { - vm_segment_set(cpu->memory, 0, 34); - mos6502_execute(cpu, 0x69); + vm_segment_set(cpu->memory, 11, 34); + vm_segment_set(cpu->memory, 10, 0x69); + cpu->PC = 10; + mos6502_execute(cpu); cr_assert_eq(cpu->A, 34); } -Test(mos6502, read_byte) -{ - vm_segment_set(cpu->memory, 0, 0x54); - cr_assert_eq(mos6502_read_byte(cpu), 0x54); -} - Test(mos6502, would_jump) { bool expect; @@ -136,6 +120,8 @@ Test(mos6502, would_jump) case BVS: case JMP: case JSR: + case RTS: + case RTI: expect = true; break; diff --git a/tests/mos6502.exec.c b/tests/mos6502.exec.c index c159a5e..88aa23d 100644 --- a/tests/mos6502.exec.c +++ b/tests/mos6502.exec.c @@ -36,10 +36,7 @@ Test(mos6502_exec, jsr) cr_assert_eq(cpu->PC, 235); - // FIXME: this behavior is wrong. People will expect the stack to - // contain 125 in this instance. This might correct execution but - // that just means that execution is the thing that's wrong! - cr_assert_eq(mos6502_pop_stack(cpu), 123); + cr_assert_eq(mos6502_pop_stack(cpu), 126); } Test(mos6502_exec, nop)