From 9bc9a22fb11d7abbbee69c93dbc14dbf09ac880d Mon Sep 17 00:00:00 2001 From: Alessio Cosenza Date: Wed, 3 Jan 2024 02:36:54 +0100 Subject: [PATCH 1/2] Thumb: Correctly emulates undefined behavior when register list is empty in block load/store This should never happen but test rom has a test for it. This commit fixes tests 227 and 229. --- emu/src/cpu/thumb/operations.rs | 37 +++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/emu/src/cpu/thumb/operations.rs b/emu/src/cpu/thumb/operations.rs index e592552..a0d7da0 100644 --- a/emu/src/cpu/thumb/operations.rs +++ b/emu/src/cpu/thumb/operations.rs @@ -4,6 +4,7 @@ use crate::cpu::arm7tdmi::Arm7tdmi; use crate::cpu::condition::Condition; use crate::cpu::flags::{LoadStoreKind, OperandKind, Operation, ReadWriteKind, ShiftKind}; use crate::cpu::registers::{REG_LR, REG_PROGRAM_COUNTER, REG_SP}; +use crate::cpu::thumb; use crate::cpu::thumb::alu_instructions::{ThumbHighRegisterOperation, ThumbModeAluInstruction}; use crate::cpu::thumb::mode::ThumbModeOpcode; use std::ops::Mul; @@ -481,15 +482,34 @@ impl Arm7tdmi { &mut self, load_store: LoadStoreKind, base_register: usize, - register_list: u16, + mut register_list: u16, ) { - let mut address = self.registers.register_at(base_register); + // According to the documentation register list should never be empty. + // In reality the THUMB tests have a test for it. What happens in this case is that + // register list will only be composed by R15 but the write back address will be the same as if + // all 16 registers are present in the register list. + + let base_address = self.registers.register_at(base_register); + let mut address = base_address; + let register_count = if register_list == 0 { + register_list = 0b1 << 15; + 16 + } else { + register_list.count_ones() + }; match load_store { LoadStoreKind::Store => { - for r in 0..=7 { + for r in 0..=15 { if register_list.is_bit_on(r) { - let value = self.registers.register_at(r as usize); + let value = self.registers.register_at(r as usize) + // If we store PC we should increase it by an instruction length. + + if r == 15 { + thumb::operations::SIZE_OF_INSTRUCTION + } else { + 0 + }; + self.bus.write_word(address as usize, value); address += 4; @@ -497,7 +517,7 @@ impl Arm7tdmi { } } LoadStoreKind::Load => { - for r in 0..=7 { + for r in 0..=15 { if register_list.get_bit(r) { let value = self.read_word(address as usize); self.registers.set_register_at(r as usize, value); @@ -508,7 +528,12 @@ impl Arm7tdmi { } } - self.registers.set_register_at(base_register, address); + self.registers + .set_register_at(base_register, base_address + register_count * 4); + + if load_store == LoadStoreKind::Load && register_list.is_bit_on(15) { + self.flush_pipeline(); + } } pub fn cond_branch(&mut self, condition: Condition, immediate_offset: i32) { From f4dc8f3428433ccf8566b4ebec5cb047b0b9f391 Mon Sep 17 00:00:00 2001 From: Alessio Cosenza Date: Wed, 3 Jan 2024 03:02:24 +0100 Subject: [PATCH 2/2] Thumb: Correctly handles base in register list for multiple load/store This commit fixes thumb tests 230,231,232. --- emu/src/cpu/thumb/operations.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/emu/src/cpu/thumb/operations.rs b/emu/src/cpu/thumb/operations.rs index a0d7da0..0bfb6d3 100644 --- a/emu/src/cpu/thumb/operations.rs +++ b/emu/src/cpu/thumb/operations.rs @@ -500,6 +500,8 @@ impl Arm7tdmi { match load_store { LoadStoreKind::Store => { + let mut first_written = false; + for r in 0..=15 { if register_list.is_bit_on(r) { let value = self.registers.register_at(r as usize) @@ -508,8 +510,17 @@ impl Arm7tdmi { thumb::operations::SIZE_OF_INSTRUCTION } else { 0 + } + // If we store the base register as second register (or later) we store + // the updated value as if it was already written back. + + if first_written && r as usize == base_register { + register_count * 4 + } else { + 0 }; + first_written = true; + self.bus.write_word(address as usize, value); address += 4;