From cc0278508eee463f0cc22f89b30cea1de435a974 Mon Sep 17 00:00:00 2001 From: Ajurna Date: Thu, 15 May 2025 16:43:42 +0100 Subject: [PATCH] Refactor CPU execution flow and improve program counter handling Introduced `ExecutionReturn` struct to standardize program counter and cycle updates across instructions. Removed redundant handling of program counter increments and refactored control flow for clarity. Also removed unused `Address` target variant from `Target` enum. --- src/instructions.rs | 1 - src/main.rs | 265 +++++++++++++++++++++++--------------------- 2 files changed, 139 insertions(+), 127 deletions(-) diff --git a/src/instructions.rs b/src/instructions.rs index 46c143c..d0fa99b 100644 --- a/src/instructions.rs +++ b/src/instructions.rs @@ -16,7 +16,6 @@ pub enum Condition { pub enum Target { U8Register(TargetRegister), U16Register(TargetU16Register), - Address(u16), Immediate(u8) } diff --git a/src/main.rs b/src/main.rs index 2312f38..3545e96 100644 --- a/src/main.rs +++ b/src/main.rs @@ -124,6 +124,7 @@ struct CPU { pc: u16, bus: MemoryBus, sp: u16, + interrupts_enabled: bool, } const BOOT_BEGIN: usize = 0x0000; @@ -204,6 +205,11 @@ impl GPU { } } + +struct ExecutionReturn { + pc: u16, + cycles: u8, +} #[derive(Debug)] struct MemoryBus { memory: [u8; 0xFFFF+1], @@ -275,8 +281,8 @@ impl CPU { } fn load_test(test: &Value) -> CPU { - let mut vram = vec![0xFFu8;VRAM_END-VRAM_BEGIN+1]; - let mut game_rom = vec![0xFFu8;CART_END+1]; + let vram = vec![0xFFu8;VRAM_END-VRAM_BEGIN+1]; + let game_rom = vec![0xFFu8;CART_END+1]; let mut memory = [0xFFu8; 0xFFFF+1]; for mem in test["ram"].as_array().unwrap() { let address = mem[0].as_u64().unwrap() as usize; @@ -317,7 +323,8 @@ impl CPU { }, boot: vec![], flat_ram: true, - } + }, + interrupts_enabled: false, } } @@ -422,7 +429,6 @@ impl CPU { let address = self.get_u16_reg_value(target_register); self.bus.read_byte(address) }, - Target::Address(address) => { self.bus.read_byte(address) }, Target::Immediate(value) => { value }, } } @@ -433,13 +439,12 @@ impl CPU { let address = self.get_u16_reg_value(target_register); self.bus.write_byte(address, value) }, - Target::Address(address) => { self.bus.write_byte(address, value) }, Target::Immediate(_) => { }, } } - - + + fn execute_next_instruction(&mut self) { // if self.pc == 0xC { // println!("clear vram Complete"); @@ -450,9 +455,31 @@ impl CPU { let inst = parse_instruction(self.bus.read_byte(self.pc), self.bus.read_byte(self.pc.wrapping_add(1)), self.bus.read_byte(self.pc.wrapping_add(2))); // println!("{:x} {:?} {:?}", self.pc, inst, self.registers.f); - self.execute(inst); + let result = self.execute(inst); + self.pc = result.pc } - fn execute(&mut self, instruction: Instruction) { + + fn standard_inc_pc_and_timing(&mut self, target: Target) -> ExecutionReturn { + match target { + Target::U8Register(_) => { + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 1} + } + Target::U16Register(_) => { + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 2} + } + Target::Immediate(_) => { + ExecutionReturn{pc: self.pc.wrapping_add(2), cycles: 3} + }, + } + } + fn rotate_inc_pc_and_timing(&mut self, target: Target) -> ExecutionReturn { + match target { + Target::U8Register(_) => {ExecutionReturn{pc: self.pc.wrapping_add(2), cycles: 2}} + Target::U16Register(_) => {ExecutionReturn{pc: self.pc.wrapping_add(2), cycles: 4}} + Target::Immediate(_) => {panic!("Does not accept immediate target")} + } + } + fn execute(&mut self, instruction: Instruction) -> ExecutionReturn { match instruction { Instruction::ADC(target) => { let value = self.get_target_value(target); @@ -464,10 +491,7 @@ impl CPU { self.registers.f.half_carry = (self.registers.a & 0xF) + (value & 0xF) + carry > 0xF; self.registers.f.carry = did_overflow | carry_overflow; self.registers.a = new_value; - self.pc += match target { - Target::Immediate(_) => {2} - _ => {1} - }; + self.standard_inc_pc_and_timing(target) } Instruction::ADD(target) => { let value = self.get_target_value(target); @@ -477,10 +501,7 @@ impl CPU { self.registers.f.carry = did_overflow; self.registers.f.half_carry = (self.registers.a & 0xF) + (value & 0xF) > 0xF; self.registers.a = new_value; - self.pc += match target { - Target::Immediate(_) => {2} - _ => {1} - }; + self.standard_inc_pc_and_timing(target) } Instruction::ADDHL(target) => { let value_source = self.get_u16_reg_value(target); @@ -490,7 +511,7 @@ impl CPU { self.registers.f.subtract = false; self.registers.f.carry = did_overflow; self.registers.f.half_carry = (value_dest & 0xFFF) + (value_source & 0xFFF) > 0xFFF; - self.pc += 1; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 2} } Instruction::ADDSP(value) => { let new_value = self.sp.wrapping_add_signed(value as i16); @@ -503,7 +524,7 @@ impl CPU { self.sp = new_value; self.registers.f.zero = false; self.registers.f.subtract = false; - self.pc += 2; + ExecutionReturn{pc: self.pc.wrapping_add(2), cycles: 4} } Instruction::AND(target) => { let value = self.get_target_value(target); @@ -512,34 +533,35 @@ impl CPU { self.registers.f.subtract = false; self.registers.f.carry = false; self.registers.f.half_carry = true; - self.pc += match target { - Target::Immediate(_) => {2} - _ => {1} - }; + self.standard_inc_pc_and_timing(target) } Instruction::BIT(bit, target) => { let value = self.get_target_value(target); self.registers.f.zero = value >> bit & 0x1 == 0; self.registers.f.subtract = false; self.registers.f.half_carry = true; - self.pc = self.pc.wrapping_add(2); + let pc = self.pc.wrapping_add(2); + match target { + Target::U8Register(_) => {ExecutionReturn{pc, cycles: 2}} + Target::U16Register(_) => {ExecutionReturn{pc, cycles: 3}} + Target::Immediate(_) => {ExecutionReturn{pc, cycles: 2}} + } } Instruction::CALL(condition, address) => { - if self.check_condition(condition) { - self.sp = self.sp.wrapping_sub(2); - let pc = self.pc+3; - self.bus.write_byte(self.sp + 1, ((pc >> 8) & 0xFF) as u8); - self.bus.write_byte(self.sp, (pc & 0xFF) as u8); - self.pc = address; + let (pc, cycles) = if self.check_condition(condition) { + let next_instruction = self.pc.wrapping_add(3); + self.push_stack(next_instruction); + (address, 6) } else { - self.pc = self.pc.wrapping_add(3); - } + (self.pc.wrapping_add(3), 3) + }; + ExecutionReturn{pc, cycles} } Instruction::CCF => { self.registers.f.subtract = false; self.registers.f.half_carry = false; self.registers.f.carry = !self.registers.f.carry; - self.pc += 1; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 1} } Instruction::CP(target) => { let value = self.get_target_value(target); @@ -549,16 +571,13 @@ impl CPU { self.registers.f.carry = did_overflow; let (_, half_carry) = (self.registers.a & 0xF).overflowing_sub(value & 0xF); self.registers.f.half_carry = half_carry; - self.pc += match target { - Target::Immediate(_) => {2} - _ => {1} - }; + self.standard_inc_pc_and_timing(target) } Instruction::CPL => { self.registers.a = !self.registers.a; self.registers.f.subtract = true; self.registers.f.half_carry = true; - self.pc += 1; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 1} } Instruction::DAA => { let new_value; @@ -571,7 +590,6 @@ impl CPU { if self.registers.f.carry { adjustment += 0x60; } - // (new_value, did_overflow) = self.registers.a.overflowing_sub(adjustment); new_value = self.registers.a.wrapping_sub(adjustment); did_overflow = self.registers.f.carry; } else { @@ -590,7 +608,7 @@ impl CPU { self.registers.f.zero = new_value == 0; self.registers.f.half_carry = false; self.registers.f.carry = did_overflow; - self.pc += 1; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 1} } Instruction::DEC(target) => { let value = self.get_target_value(target); @@ -600,22 +618,24 @@ impl CPU { self.registers.f.subtract = true; let (_, half_carry) = (value & 0xF).overflowing_sub(1); self.registers.f.half_carry = half_carry; - self.pc = self.pc.wrapping_add(1); + self.standard_inc_pc_and_timing(target) } Instruction::DECU16(register) => { let value = self.get_u16_reg_value(register); let new_value = value.wrapping_sub(1); self.set_u16_reg_value(register, new_value); - self.pc = self.pc.wrapping_add(1); + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 1} } Instruction::DI => { - self.pc += 1; + self.interrupts_enabled = false; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 1} } Instruction::EI => { - self.pc += 1; + self.interrupts_enabled = true; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 1} } Instruction::HALT => { - self.pc += 1; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 1} } Instruction::INC(target) => { @@ -625,35 +645,30 @@ impl CPU { self.registers.f.zero = new_value == 0; self.registers.f.subtract = false; self.registers.f.half_carry = (value & 0xF) + 1 > 0xF; - self.pc += 1; + self.standard_inc_pc_and_timing(target) } Instruction::INCU16(register) => { let value = self.get_u16_reg_value(register); let new_value = value.wrapping_add(1); self.set_u16_reg_value(register, new_value); - self.pc += 1; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 2} } Instruction::JP(condition, address) => { if self.check_condition(condition) { self.pc = address; + ExecutionReturn{pc: address, cycles: 4} } else { - self.pc = self.pc.wrapping_add(3); + ExecutionReturn{pc: self.pc.wrapping_add(3), cycles: 3} } } Instruction::JPHL => { - self.pc = self.registers.get_hl(); + ExecutionReturn{pc: self.registers.get_hl(), cycles: 1} } Instruction::JR(condition, offset) => { if self.check_condition(condition) { - self.pc = if offset.is_negative() { - let t = self.pc.wrapping_sub((offset as i16).abs() as u16); - t.wrapping_add(2) - } else { - let t = self.pc.wrapping_add(offset as u16); - t.wrapping_add(2) - } - } else { - self.pc += 2; + ExecutionReturn{pc: self.pc.wrapping_add_signed(offset as i16).wrapping_add(2), cycles: 4} + } else { + ExecutionReturn{pc: self.pc.wrapping_add(2), cycles: 2} } } Instruction::LD(target) => { @@ -661,100 +676,94 @@ impl CPU { LoadTarget::CopyR8R8(dest_register, source_register) => { let value = self.get_u8_reg_value(source_register); self.set_u8_reg_value(dest_register, value); - self.pc = self.pc.wrapping_add(1); + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 1} } LoadTarget::CopyR8N8(dest_register, value) => { self.set_u8_reg_value(dest_register, value); - self.pc += 2; + ExecutionReturn{pc: self.pc.wrapping_add(2), cycles: 2} } LoadTarget::CopyR16N16(dest_register, value) => { self.set_u16_reg_value(dest_register, value); - self.pc += 3; + ExecutionReturn{pc: self.pc.wrapping_add(3), cycles: 3} } LoadTarget::CopyHLR8(source_register) => { let value = self.get_u8_reg_value(source_register); let address = self.registers.get_hl(); self.bus.write_byte(address, value); - self.pc = self.pc.wrapping_add(1); + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 2} } LoadTarget::CopyHLN8(value) => { let address = self.registers.get_hl(); self.bus.write_byte(address, value); - self.pc += 2; + ExecutionReturn{pc: self.pc.wrapping_add(2), cycles: 3} } LoadTarget::CopyR8HL(dest_register) => { let address = self.registers.get_hl(); let value = self.bus.read_byte(address); self.set_u8_reg_value(dest_register, value); - self.pc += 1; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 2} } LoadTarget::CopyR16A(dest_register) => { let address = self.get_u16_reg_value(dest_register); self.bus.write_byte(address, self.registers.a); - self.pc += 1; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 2} } LoadTarget::CopyN16A(address) => { self.bus.write_byte(address, self.registers.a); - self.pc += 3; + ExecutionReturn{pc: self.pc.wrapping_add(3), cycles: 4} } LoadTarget::CopyPortA(target) => { let offset = self.get_target_value(target); let address = 0xFF00 | offset as u16; self.bus.write_byte(address, self.registers.a); - self.pc = match target { - Target::Immediate(_) => {self.pc.wrapping_add(2)}, - _ => {self.pc.wrapping_add(1)} - }; + self.standard_inc_pc_and_timing(target) } LoadTarget::CopyAPort(target) => { let offset = self.get_target_value(target); let address = 0xFF00 | offset as u16; self.registers.a = self.bus.read_byte(address); - self.pc = match target { - Target::Immediate(_) => {self.pc.wrapping_add(2)} - _ => {self.pc.wrapping_add(1)} - }; + self.standard_inc_pc_and_timing(target) } LoadTarget::CopyAR16(source_register) => { let address = self.get_u16_reg_value(source_register); self.registers.a = self.bus.read_byte(address); - self.pc += 1; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 2} } LoadTarget::CopyAN16(address) => { self.registers.a = self.bus.read_byte(address); - self.pc += 3; + ExecutionReturn{pc: self.pc.wrapping_add(3), cycles: 4} } LoadTarget::CopyHLIA => { let address = self.registers.get_hl(); self.bus.write_byte(address, self.registers.a); self.registers.set_hl(address.wrapping_add(1)); - self.pc += 1; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 2} } LoadTarget::CopyHLDA => { let address = self.registers.get_hl(); self.bus.write_byte(address, self.registers.a); self.registers.set_hl(address.wrapping_sub(1)); - self.pc += 1; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 2} } LoadTarget::CopyAHLI => { let address = self.registers.get_hl(); self.registers.a = self.bus.read_byte(address); self.registers.set_hl(address.wrapping_add(1)); - self.pc += 1; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 2} } LoadTarget::CopyAHLD => { let address = self.registers.get_hl(); self.registers.a = self.bus.read_byte(address); self.registers.set_hl(address.wrapping_sub(1)); - self.pc += 1; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 2} } LoadTarget::CopySPN16(value) => { self.sp = value; - self.pc = self.pc.wrapping_add(3); + ExecutionReturn{pc: self.pc.wrapping_add(3), cycles: 3} } LoadTarget::CopyN16SP(address) => { self.bus.write_u16(address, self.sp); - self.pc += 3; + ExecutionReturn{pc: self.pc.wrapping_add(3), cycles: 5} } LoadTarget::CopyHLSPE8(value) => { let new_value = self.sp.wrapping_add_signed(value as i16); @@ -765,16 +774,16 @@ impl CPU { self.registers.f.subtract = false; self.registers.f.carry = carry; self.registers.f.half_carry = half_carry; - self.pc += 2; + ExecutionReturn{pc: self.pc.wrapping_add(2), cycles: 3} } LoadTarget::CopySPHL => { self.sp = self.registers.get_hl(); - self.pc += 1; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 2} } } } Instruction::NOP => { - self.pc += 1; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 1} } Instruction::OR(target) => { let value = self.get_target_value(target); @@ -783,36 +792,40 @@ impl CPU { self.registers.f.subtract = false; self.registers.f.carry = false; self.registers.f.half_carry = false; - self.pc += match target { - Target::Immediate(_) => {2} - _ => {1} - }; + self.standard_inc_pc_and_timing(target) } Instruction::POP(target) => { let value = self.pop_stack(); self.set_u16_reg_value(target, value); - self.pc += 1; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 3} } Instruction::PUSH(target) => { let value = self.get_u16_reg_value(target); self.push_stack(value); - self.pc += 1; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 4} } Instruction::RES(bit, target) => { let mut value = self.get_target_value(target); value &= !(1 << bit); self.set_target_value(target, value); - self.pc = self.pc.wrapping_add(2); + match target { + Target::U8Register(_) => {ExecutionReturn{pc: self.pc.wrapping_add(2), cycles: 2}} + _ => {ExecutionReturn{pc: self.pc.wrapping_add(2), cycles: 4}} + } } Instruction::RET(condition) => { if self.check_condition(condition) { - self.pc = self.pop_stack(); + match condition { + Condition::None => {ExecutionReturn{pc: self.pop_stack(), cycles: 4}}, + _ => {ExecutionReturn{pc: self.pop_stack(), cycles: 5}} + } } else { - self.pc += 1; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 2} } } Instruction::RETI => { - self.pc = self.pop_stack(); + self.interrupts_enabled = true; + ExecutionReturn{pc: self.pop_stack(), cycles: 4} } Instruction::RL(target) => { let old_value = self.get_target_value(target); @@ -825,11 +838,11 @@ impl CPU { match self.bus.read_byte(self.pc) { 0x17 => { self.registers.f.zero = false; - self.pc = self.pc.wrapping_add(1); + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 1} } _ => { self.registers.f.zero = new_value == 0; - self.pc = self.pc.wrapping_add(2); + self.rotate_inc_pc_and_timing(target) } } } @@ -840,14 +853,14 @@ impl CPU { self.set_target_value(target, new_value); self.registers.f.subtract = false; self.registers.f.half_carry = false; - self.pc += match self.bus.read_byte(self.pc) { + match self.bus.read_byte(self.pc) { 0x07 => { self.registers.f.zero = false; - 1 + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 1} } _ => { self.registers.f.zero = new_value == 0; - 2 + self.rotate_inc_pc_and_timing(target) } } } @@ -859,14 +872,14 @@ impl CPU { self.set_target_value(target, new_value); self.registers.f.subtract = false; self.registers.f.half_carry = false; - self.pc += match self.bus.read_byte(self.pc) { + match self.bus.read_byte(self.pc) { 0x1F => { self.registers.f.zero = false; - 1 + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 1} } _ => { self.registers.f.zero = new_value == 0; - 2 + self.rotate_inc_pc_and_timing(target) } } } @@ -877,20 +890,20 @@ impl CPU { self.set_target_value(target, new_value); self.registers.f.subtract = false; self.registers.f.half_carry = false; - self.pc += match self.bus.read_byte(self.pc) { + match self.bus.read_byte(self.pc) { 0x0F => { self.registers.f.zero = false; - 1 + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 1} } _ => { self.registers.f.zero = new_value == 0; - 2 + self.rotate_inc_pc_and_timing(target) } } } Instruction::RST(idx) => { self.push_stack(self.pc.wrapping_add(1)); - self.pc = idx as u16 * 8; + ExecutionReturn{pc: idx as u16 * 8, cycles: 4} } Instruction::SBC(target) => { let value = self.get_target_value(target); @@ -904,22 +917,23 @@ impl CPU { let (_, half_carry2) = temp.overflowing_sub(value & 0xF); self.registers.f.half_carry = half_carry | half_carry2; self.registers.a = new_value; - self.pc += match target { - Target::Immediate(_) => {2} - _ => {1} - }; + self.standard_inc_pc_and_timing(target) } Instruction::SCF => { self.registers.f.carry = true; self.registers.f.subtract = false; self.registers.f.half_carry = false; - self.pc += 1; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 1} } Instruction::SET(bit, target) => { let mut value = self.get_target_value(target); value |= 1 << bit; self.set_target_value(target, value); - self.pc = self.pc.wrapping_add(2); + match target { + Target::U8Register(_) => {ExecutionReturn{pc: self.pc.wrapping_add(2), cycles: 2}} + Target::U16Register(_) => {ExecutionReturn{pc: self.pc.wrapping_add(2), cycles: 4}} + Target::Immediate(_) => {panic!("Immediate not supported")} + } } Instruction::SLA(target) => { let value = self.get_target_value(target); @@ -929,7 +943,7 @@ impl CPU { self.registers.f.subtract = false; self.registers.f.carry = value & 0x80 == 0x80; self.registers.f.half_carry = false; - self.pc = self.pc.wrapping_add(2); + self.rotate_inc_pc_and_timing(target) } Instruction::SRA(target) => { let value = self.get_target_value(target); @@ -939,7 +953,7 @@ impl CPU { self.registers.f.subtract = false; self.registers.f.carry = value & 0x1 == 0x1; self.registers.f.half_carry = false; - self.pc += 2; + self.rotate_inc_pc_and_timing(target) } Instruction::SRL(target) => { let value = self.get_target_value(target); @@ -949,10 +963,10 @@ impl CPU { self.registers.f.subtract = false; self.registers.f.carry = value & 0x1 == 0x1; self.registers.f.half_carry = false; - self.pc = self.pc.wrapping_add(2); + self.rotate_inc_pc_and_timing(target) } Instruction::STOP(_) => { - self.pc += 1; + ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 1} } Instruction::SUB(target) => { let value = self.get_target_value(target); @@ -962,10 +976,7 @@ impl CPU { self.registers.f.subtract = true; self.registers.f.carry = did_overflow; self.registers.f.half_carry = (self.registers.a & 0xF) + (value & 0xF) > 0xF; - self.pc += match target { - Target::Immediate(_) => {2} - _ => {1} - }; + self.standard_inc_pc_and_timing(target) } Instruction::SWAP(target) => { let value = self.get_target_value(target); @@ -975,7 +986,7 @@ impl CPU { self.registers.f.subtract = false; self.registers.f.carry = false; self.registers.f.half_carry = false; - self.pc = self.pc.wrapping_add(2); + self.rotate_inc_pc_and_timing(target) } Instruction::XOR(target) => { let value = self.get_target_value(target); @@ -984,10 +995,11 @@ impl CPU { self.registers.f.subtract = false; self.registers.f.carry = false; self.registers.f.half_carry = false; - self.pc += match target { - Target::Immediate(_) => {2} - _ => {1} - }; + match target { + Target::U8Register(_) => {ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 1}} + Target::U16Register(_) => {ExecutionReturn{pc: self.pc.wrapping_add(1), cycles: 2}} + Target::Immediate(_) => {ExecutionReturn{pc: self.pc.wrapping_add(2), cycles: 2}} + } } } } @@ -1029,6 +1041,7 @@ fn run_gameboy() { flat_ram: false }, sp: 0, + interrupts_enabled: false, }; gameboy.init(); let mut count =0;