diff --git a/src/cpu.rs b/src/cpu.rs index a04a38b..4c5bdfa 100644 --- a/src/cpu.rs +++ b/src/cpu.rs @@ -33,8 +33,10 @@ impl Processor { let (instruction, bytes_read) = parse::next_instruction(&memory_view).expect("invalid instruction"); + // Must advance the PC before running the instruction to ensure CALLs/JUMPs operate on the correct addresses + self.registers.program_counter = pc.wrapping_add(bytes_read); + let interrupt_enable_pending_already = self.interrupt_enable_pending; - let pc_before_instruction_run = self.registers.program_counter; self.run_instruction(instruction); // EI does not work until after the following instruction, so we check after running an instruction if @@ -43,13 +45,6 @@ impl Processor { self.interrupt_enable_pending = false; self.registers.enable_interrupts(); } - - if pc_before_instruction_run == self.registers.program_counter { - // The program counter should be advanced *ONLY* if the instruction did not modify the PC register - - let next_pc = pc.wrapping_add(bytes_read); - self.registers.program_counter = next_pc; - } } /// Run a single instruction on the CPU. diff --git a/src/cpu/run/control.rs b/src/cpu/run/control.rs index 588d280..3718147 100644 --- a/src/cpu/run/control.rs +++ b/src/cpu/run/control.rs @@ -49,8 +49,7 @@ impl Run for ControlFlowInstruction { .registers .get_single_16bit_register(register::SingleSixteenBit::ProgramCounter); - // We know the rest instruction to be 1 byte. A bit of a hack, but eh. - processor.push_16bit_value_to_stack(current_pc.wrapping_add(1))?; + processor.push_16bit_value_to_stack(current_pc)?; processor .registers @@ -89,8 +88,7 @@ fn do_call(processor: &mut Processor, to_addr: u16) -> Result<(), super::Error> .registers .get_single_16bit_register(register::SingleSixteenBit::ProgramCounter); - // We know the call instruction to be 3 bytes. A bit of a hack, but eh. - processor.push_16bit_value_to_stack(current_pc.wrapping_add(3))?; + processor.push_16bit_value_to_stack(current_pc)?; jump_to(processor, to_addr); diff --git a/tests/cpu/control.rs b/tests/cpu/control.rs index d47debf..53ec47a 100644 --- a/tests/cpu/control.rs +++ b/tests/cpu/control.rs @@ -64,8 +64,9 @@ fn test_call_pushes_pc_onto_stack() { processor.run_instruction(ins); assert_eq!(0xBE, processor.memory.get((old_sp - 1).into()).unwrap()); - // 0xEF + 3, which is the size of the CALL instruction - assert_eq!(0xF2, processor.memory.get((old_sp - 2).into()).unwrap()); + // HACK: This SHOULD be F2 in reality (0xEF + 3, the size of the call instruction) + // however since run_instruction does not advance the PC, we need to adjust our test + assert_eq!(0xEF, processor.memory.get((old_sp - 2).into()).unwrap()); assert_eq!(old_sp - 2, processor.registers.stack_pointer); } @@ -111,8 +112,9 @@ fn test_restart_adjusts_pushes_current_program_counter_to_stack(opcode: u8) { processor.run_instruction(ins); assert_eq!(0xDE, processor.memory.get((old_sp - 1).into()).unwrap()); - // 0xAD + 1, which is the size of the RST instruction - assert_eq!(0xAE, processor.memory.get((old_sp - 2).into()).unwrap()); + // HACK: This SHOULD be AE in reality (0xAD + 1, the size of the rst instruction) + // however since run_instruction does not advance the PC, we need to adjust our test + assert_eq!(0xAD, processor.memory.get((old_sp - 2).into()).unwrap()); assert_eq!(old_sp - 2, processor.registers.stack_pointer); } @@ -185,6 +187,29 @@ fn test_call_and_jump_does_nothing_if_condition_fails(opcode: u8, flag: Flag, ex assert_eq!(old_pc, processor.registers.program_counter); } +#[test] +fn test_jump_to_same_addr_does_not_advance_pc() { + let mut processor = Processor::default(); + processor.registers.program_counter = 0x1337; + + [0xC3, 0x37, 0x13] + .iter() + .copied() + .enumerate() + .for_each(|(idx, opcode)| { + processor + .memory + .set( + usize::from(processor.registers.program_counter) + idx, + opcode, + ) + .expect("could not program data"); + }); + + processor.run_next_instruction(); + assert_eq!(0x1337, processor.registers.program_counter); +} + #[test_case(0xC9; "RET")] #[test_case(0xD9; "RETI")] fn test_ret_jumps_to_address_on_stack(opcode: u8) {