Refactor increment cycles function

snes-core/src/cpu/cycles.rs

@@ -1,68 +1,108 @@
 use super::cpu::CPU;
 use crate::utils::addressing::{AddressingMode, IndexRegister};
 
+type A = AddressingMode;
+type I = IndexRegister;
+
+enum Condition {
+    MemorySelectFlag,
+    DirectPageZero,
+    IndexCrossesPageBoundary,
+    DecimalMode,
+}
+
+const ALL_CONDITIONS: [Condition; 4] = [
+    Condition::MemorySelectFlag,
+    Condition::DirectPageZero,
+    Condition::IndexCrossesPageBoundary,
+    Condition::DecimalMode,
+];
 
 impl CPU {
-    pub fn increment_cycles_adc_sbc(&mut self, addressing_mode: AddressingMode) {
+    fn common_conditions(&mut self, addressing_mode: AddressingMode, conditions: &[Condition]) {
-        type A = AddressingMode;
+        let mut bytes = 0;
-        type I = IndexRegister;
+        let mut cycles = 0;
-        let (mut bytes, mut cycles) = match addressing_mode {
-            A::Immediate => (2, 2),
-            A::Absolute => (3, 4),
-            A::AbsoluteLong => (4, 5),
-            A::DirectPage => (2, 3),
-            A::DirectPageIndirect => (2, 5),
-            A::DirectPageIndirectLong => (2, 6),
-            A::AbsoluteIndexed(_) => (3, 4),
-            A::AbsoluteLongIndexed(_) => (4, 5),
-            A::DirectPageIndexed(_) => (2, 4),
-            A::DirectPageIndexedIndirect(_) => (2, 6),
-            A::DirectPageIndirectIndexed(_) => (2, 5),
-            A::DirectPageIndirectLongIndexed(_) => (2, 6),
-            A::StackRelative => (2, 4),
-            A::StackRelativeIndirectIndexed(_) => (2, 7),
-        };
-        // condition 1: Add 1 byte and 1 cycle if m = 0 (16-bit memory/accumulator)
-        if !self.registers.get_memory_select_flag() {
-            bytes += 1;
-            cycles += 1;
-        }
 
-        // condition 2: Add 1 cycle if low byte of Direct Page register is other than zero (DL< >0)
+        for condition in conditions {
-        match addressing_mode {
+            match condition {
-            A::DirectPage | A::DirectPageIndirect | A::DirectPageIndirectLong |
+                // Add 1 byte and 1 cycle if m = 0 (16-bit memory/accumulator)
-            A::DirectPageIndexed(_) | A::DirectPageIndexedIndirect(_) |
+                Condition::MemorySelectFlag => {
-            A::DirectPageIndirectIndexed(_) | A::DirectPageIndirectLongIndexed(_) => {
+                    if !self.registers.get_memory_select_flag() {
-                if self.registers.direct_page_low() != 0 {
+                        cycles += 1;
-                    cycles += 1;
+                        match addressing_mode {
-                }
+                            A::Immediate => bytes += 1,
-            },
+                            _ => {},
-            _ => {},
+                        }
-        };
+                    }
-
+                },
-        // condition 3: Add 1 cycle if adding index crosses a page boundary
+                // Add 1 cycle if low byte of Direct Page register is other than zero (DL< >0)
-        match addressing_mode {
+                Condition::DirectPageZero => {
-            A::AbsoluteIndexed(index) | A::DirectPageIndirectIndexed(index) => {
+                    match addressing_mode {
-                let page = (self.registers.get_pc_address() - (bytes as u32)) & 0xFF;
+                        A::DirectPage | A::DirectPageIndirect | A::DirectPageIndirectLong |
-                let index = match index {
+                        A::DirectPageIndexed(_) | A::DirectPageIndexedIndirect(_) |
-                    I::X => self.registers.x,
+                        A::DirectPageIndirectIndexed(_) |
-                    I::Y => self.registers.y,
+                        A::DirectPageIndirectLongIndexed(_) => {
-                };
+                            if self.registers.direct_page_low() != 0 {
-                if (page + index as u32) > 0xFF {
+                                cycles += 1;
-                    cycles += 1
+                            }
-                }
+                        },
-            },
+                        _ => {},
-            _ => {},
+                    };
-        };
+                },
-
+                // Add 1 cycle if adding index crosses a page boundary
-        // condition 4: Add 1 cycle if 65C02 and d = 1 (decimal mode, 65C02)
+                Condition::IndexCrossesPageBoundary => {
-        if self.registers.get_decimal_mode_flag() {
+                    match addressing_mode {
-            cycles += 1;
+                        A::AbsoluteIndexed(index) | A::DirectPageIndirectIndexed(index) => {
+                            let page = self.registers.get_pc_address() & 0xFF;
+                            let index = match index {
+                                I::X => self.registers.x,
+                                I::Y => self.registers.y,
+                            };
+                            if (page + index as u32) > 0xFF {
+                                cycles += 1
+                            }
+                        },
+                        _ => {},
+                    };
+                },
+                // Add 1 cycle if 65C02 and d = 1 (decimal mode, 65C02)
+                Condition::DecimalMode => {
+                    if self.registers.get_decimal_mode_flag() {
+                        cycles += 1;
+                    }
+                },
+            };
         }
 
         self.registers.increment_pc(bytes);
         self.cycles += cycles;
     }
+
+    fn common_bytes_cycles_arithmetic(addressing_mode: AddressingMode) -> (u16, usize) {
+        match addressing_mode {
+            A::Immediate                        => (2, 2),
+            A::Absolute                         => (3, 4),
+            A::AbsoluteLong                     => (4, 5),
+            A::DirectPage                       => (2, 3),
+            A::DirectPageIndirect               => (2, 5),
+            A::DirectPageIndirectLong           => (2, 6),
+            A::AbsoluteIndexed(_)               => (3, 4),
+            A::AbsoluteLongIndexed(_)           => (4, 5),
+            A::DirectPageIndexed(_)             => (2, 4),
+            A::DirectPageIndexedIndirect(_)     => (2, 6),
+            A::DirectPageIndirectIndexed(_)     => (2, 5),
+            A::DirectPageIndirectLongIndexed(_) => (2, 6),
+            A::StackRelative                    => (2, 4),
+            A::StackRelativeIndirectIndexed(_)  => (2, 7),
+        }
+    }
+
+    pub fn increment_cycles_arithmetic(&mut self, addressing_mode: AddressingMode) {
+        let (bytes, cycles) = CPU::common_bytes_cycles_arithmetic(addressing_mode);
+        self.registers.increment_pc(bytes);
+        self.cycles += cycles;
+        self.common_conditions(addressing_mode, &ALL_CONDITIONS);
+    }
 }
 
 #[cfg(test)]
@@ -70,7 +110,7 @@ mod cpu_instructions_tests {
     use super::*;
 
     #[test]
-    fn test_increment_cycles_adc_sbc() {
+    fn test_common_conditions() {
         let mut cpu = CPU::new();
 
         // 16-bit Memory/accumulator flag condition
@@ -78,25 +118,25 @@ mod cpu_instructions_tests {
         cpu.cycles = 0;
         cpu.registers.set_decimal_mode_flag(false);
         cpu.registers.set_16bit_mode(false);
-        cpu.increment_cycles_adc_sbc(AddressingMode::Immediate);
+        cpu.common_conditions(AddressingMode::Immediate, &ALL_CONDITIONS);
-        assert_eq!(cpu.registers.pc, 2);
+        assert_eq!(cpu.registers.pc, 0);
-        assert_eq!(cpu.cycles, 2);
+        assert_eq!(cpu.cycles, 0);
 
         cpu.registers.pc = 0;
         cpu.cycles = 0;
         cpu.registers.set_16bit_mode(true);
-        cpu.increment_cycles_adc_sbc(AddressingMode::Immediate);
+        cpu.common_conditions(AddressingMode::Immediate, &ALL_CONDITIONS);
-        assert_eq!(cpu.registers.pc, 3);
+        assert_eq!(cpu.registers.pc, 1);
-        assert_eq!(cpu.cycles, 3);
+        assert_eq!(cpu.cycles, 1);
 
         // Decimal flag condition
         cpu.registers.pc = 0;
         cpu.cycles = 0;
         cpu.registers.set_16bit_mode(true);
         cpu.registers.set_decimal_mode_flag(true);
-        cpu.increment_cycles_adc_sbc(AddressingMode::Immediate);
+        cpu.common_conditions(AddressingMode::Immediate, &ALL_CONDITIONS);
-        assert_eq!(cpu.registers.pc, 3);
+        assert_eq!(cpu.registers.pc, 1);
-        assert_eq!(cpu.cycles, 4);
+        assert_eq!(cpu.cycles, 2);
 
         // Low byte of direct page register other than zero condition
         cpu.registers.pc = 0;
@@ -104,18 +144,18 @@ mod cpu_instructions_tests {
         cpu.registers.set_16bit_mode(false);
         cpu.registers.set_decimal_mode_flag(false);
         cpu.registers.d = 0x0000;
-        cpu.increment_cycles_adc_sbc(AddressingMode::DirectPage);
+        cpu.common_conditions(AddressingMode::DirectPage, &ALL_CONDITIONS);
-        assert_eq!(cpu.registers.pc, 2);
+        assert_eq!(cpu.registers.pc, 0);
-        assert_eq!(cpu.cycles, 3);
+        assert_eq!(cpu.cycles, 0);
 
         cpu.registers.pc = 0;
         cpu.cycles = 0;
         cpu.registers.set_16bit_mode(false);
         cpu.registers.set_decimal_mode_flag(false);
         cpu.registers.d = 0x0001;
-        cpu.increment_cycles_adc_sbc(AddressingMode::DirectPage);
+        cpu.common_conditions(AddressingMode::DirectPage, &ALL_CONDITIONS);
-        assert_eq!(cpu.registers.pc, 2);
+        assert_eq!(cpu.registers.pc, 0);
-        assert_eq!(cpu.cycles, 4);
+        assert_eq!(cpu.cycles, 1);
 
         // Adding index crosses a page boundary condition
         cpu.registers.pc = 0xFE;
@@ -123,17 +163,27 @@ mod cpu_instructions_tests {
         cpu.cycles = 0;
         cpu.registers.set_16bit_mode(false);
         cpu.registers.set_decimal_mode_flag(false);
-        cpu.increment_cycles_adc_sbc(AddressingMode::AbsoluteIndexed(IndexRegister::X));
+        cpu.common_conditions(AddressingMode::AbsoluteIndexed(IndexRegister::X), &ALL_CONDITIONS);
-        assert_eq!(cpu.registers.pc, 0xFE + 3);
+        assert_eq!(cpu.registers.pc, 0xFE);
-        assert_eq!(cpu.cycles, 4);
+        assert_eq!(cpu.cycles, 0); // Doesn't cross boundary
 
         cpu.registers.pc = 0xFE;
         cpu.registers.x = 0x0010;
         cpu.cycles = 0;
         cpu.registers.set_16bit_mode(false);
         cpu.registers.set_decimal_mode_flag(false);
-        cpu.increment_cycles_adc_sbc(AddressingMode::AbsoluteIndexed(IndexRegister::X));
+        cpu.common_conditions(AddressingMode::AbsoluteIndexed(IndexRegister::X), &ALL_CONDITIONS);
-        assert_eq!(cpu.registers.pc, 0xFE + 3);
+        assert_eq!(cpu.registers.pc, 0xFE);
+        assert_eq!(cpu.cycles, 1); // Crosses boundary
+
+        // Test common and aritmetic together
+        cpu.registers.pc = 0xF5;
+        cpu.registers.x = 0x0010;
+        cpu.cycles = 0;
+        cpu.registers.set_16bit_mode(false);
+        cpu.registers.set_decimal_mode_flag(false);
+        cpu.increment_cycles_arithmetic(AddressingMode::AbsoluteIndexed(IndexRegister::X));
+        assert_eq!(cpu.registers.pc, 0xF5 + 3);
         assert_eq!(cpu.cycles, 5);
     }
 }

snes-core/src/cpu/instructions.rs

@@ -55,6 +55,7 @@ impl CPU {
                 self.registers.set_zero_flag(is_zero);
             }
         };
+        self.increment_cycles_arithmetic(addressing_mode);
     }
 
     fn sbc(&mut self, bus: &Bus, addressing_mode: AddressingMode) {
@@ -88,6 +89,7 @@ impl CPU {
                 self.registers.set_zero_flag(is_zero);
             }
         };
+        self.increment_cycles_arithmetic(addressing_mode);
     }
 
     pub fn execute_opcode(&mut self, opcode: u8, bus: &Bus) {
@@ -146,6 +148,8 @@ mod cpu_instructions_tests {
         bus.write(0x000001, 0x40);
         cpu.adc(&bus, AddressingMode::Immediate);
         assert_eq!(cpu.registers.a, 0x40);
+        assert_eq!(cpu.registers.pc, 0x02);
+        assert_eq!(cpu.cycles, 2);
         assert!(!cpu.registers.get_carry_flag());
     }
 
@@ -160,6 +164,8 @@ mod cpu_instructions_tests {
         bus.write(0x000001, 1);
         cpu.sbc(&bus, AddressingMode::Immediate);
         assert_eq!(cpu.registers.a, 0);
+        assert_eq!(cpu.registers.pc, 0x02);
+        assert_eq!(cpu.cycles, 2);
         assert!(!cpu.registers.get_carry_flag());
         assert!(cpu.registers.get_zero_flag());
     }