diff --git a/crates/gba-core/src/cpu.rs b/crates/gba-core/src/cpu.rs index 11c0423..99aa904 100644 --- a/crates/gba-core/src/cpu.rs +++ b/crates/gba-core/src/cpu.rs @@ -28,6 +28,8 @@ mod status_tests; #[cfg(test)] mod tests; #[cfg(test)] +mod thumb_empty_tests; +#[cfg(test)] mod thumb_tests; #[cfg(test)] mod timing_tests; diff --git a/crates/gba-core/src/cpu/thumb_empty_tests.rs b/crates/gba-core/src/cpu/thumb_empty_tests.rs new file mode 100644 index 0000000..36b1df7 --- /dev/null +++ b/crates/gba-core/src/cpu/thumb_empty_tests.rs @@ -0,0 +1,173 @@ +//! Original regressions for the stored PC in Thumb empty-register-list transfers. +use super::*; +use crate::{ + io::TIMER_BASE, + machine::{Machine, MachineError, StepKind}, + memory::ROM_START, + timing::StepTiming, +}; + +const CODE: u32 = 0x0300_0200; +const DATA: u32 = 0x0200_1000; + +fn setup(instruction: u16, pc: u32) -> (Cpu, Memory) { + let mut memory = Memory::new(vec![0; 4]).unwrap(); + memory.write16(pc, instruction).unwrap(); + let mut cpu = Cpu::new(pc); + cpu.instruction_set = InstructionSet::Thumb; + (cpu, memory) +} + +#[test] +fn empty_thumb_stores_use_pc_plus_six_for_all_bases_banks_and_alignments() { + for base_register in (0..8).chain(std::iter::once(13)) { + let push = base_register == 13; + let instruction = if push { + 0xb400 + } else { + 0xc000 | (base_register as u16) << 8 + }; + for mode in [ + Mode::User, + Mode::System, + Mode::Fiq, + Mode::Irq, + Mode::Supervisor, + Mode::Abort, + Mode::Undefined, + ] { + for code_low in [0, 2] { + for data_low in 0..4 { + for timed in [false, true] { + let pc = CODE + code_low; + let (mut cpu, mut memory) = setup(instruction, pc); + cpu.apply_status(0xb000_00e0 | mode as u32, mode); + for reg in 0..15 { + cpu.registers[reg] = 0x1000 + reg as u32; + } + let base = DATA + data_low + if push { 64 } else { 0 }; + cpu.registers[base_register] = base; + memory.write32(DATA - 4, 0x1122_3344).unwrap(); + memory.write32(DATA + 4, 0x5566_7788).unwrap(); + memory.write32(DATA + 60, 0x99aa_bbcc).unwrap(); + let mut after = cpu.clone(); + after.registers[base_register] = if push { base - 64 } else { base + 64 }; + after.registers[15] = pc + 2; + if timed { + assert_eq!( + cpu.step_timed(&mut memory).unwrap(), + StepTiming { + code_cycles: 1, + data_cycles: 6, + internal_cycles: 0, + idle_cycles: 0, + } + ); + } else { + cpu.step(&mut memory).unwrap(); + } + assert_eq!( + cpu, after, + "{instruction:#06x} {mode:?} code={code_low} data={data_low}" + ); + assert_eq!(memory.read32(DATA).unwrap(), pc + 6); + assert_eq!(memory.read32(DATA - 4).unwrap(), 0x1122_3344); + assert_eq!(memory.read32(DATA + 4).unwrap(), 0x5566_7788); + assert_eq!(memory.read32(DATA + 60).unwrap(), 0x99aa_bbcc); + assert_eq!(memory.cycles(), 0); + } + } + } + } + } +} + +#[test] +fn empty_store_pc_matches_the_following_thumb_pc_operand() { + let (mut cpu, mut memory) = setup(0xc300, CODE); // STMIA r3!, {} + memory.write16(CODE + 2, 0x467c).unwrap(); // MOV r4, pc + memory.write16(CODE + 4, 0x6835).unwrap(); // LDR r5, [r6] + memory.write16(CODE + 6, 0x42ac).unwrap(); // CMP r4, r5 + cpu.registers[3] = DATA; + cpu.registers[6] = DATA; + for _ in 0..4 { + cpu.step(&mut memory).unwrap(); + } + assert_eq!(cpu.registers[3], DATA + 64); + assert_eq!(cpu.registers[4], CODE + 6); + assert_eq!(cpu.registers[5], CODE + 6); + assert!(cpu.flags.zero && cpu.flags.carry); + assert_eq!(cpu.pc(), CODE + 8); +} + +#[test] +fn empty_store_uses_the_executing_rom_address_without_word_alignment() { + for offset in [0, 2] { + let mut rom = vec![0; 4]; + rom[offset..offset + 2].copy_from_slice(&0xc700u16.to_le_bytes()); + for window in [ROM_START, 0x0a00_0000, 0x0c00_0000] { + let mut memory = Memory::new(rom.clone()).unwrap(); + let mut cpu = Cpu::new(window + offset as u32); + cpu.instruction_set = InstructionSet::Thumb; + cpu.registers[7] = DATA; + cpu.step(&mut memory).unwrap(); + assert_eq!(memory.read32(DATA).unwrap(), window + offset as u32 + 6); + } + } +} + +#[test] +fn empty_store_does_not_validate_the_writeback_address() { + let (mut cpu, mut memory) = setup(0xc000, CODE); + cpu.registers[0] = 0x03ff_ffff; // Last mirrored IWRAM word; writeback crosses into I/O. + cpu.step(&mut memory).unwrap(); + assert_eq!(memory.read32(0x03ff_fffc).unwrap(), CODE + 6); + assert_eq!(cpu.registers[0], 0x0400_003f); + assert_eq!(cpu.pc(), CODE + 2); +} + +#[test] +fn empty_stores_advance_devices_only_after_a_successful_single_word_write() { + let (mut cpu, mut memory) = setup(0xc200, CODE); + cpu.registers[2] = DATA; + memory.write32(TIMER_BASE, 0x0080_0000).unwrap(); + let mut machine = Machine::new(cpu, memory); + assert_eq!(machine.step().unwrap(), StepKind::Instruction); + assert_eq!(machine.cycles(), 7); // One IWRAM code access and one EWRAM word store. + assert_eq!(machine.memory().read16(TIMER_BASE).unwrap(), 7); + assert_eq!(machine.memory().read32(DATA).unwrap(), CODE + 6); + assert_eq!(machine.cpu().registers[2], DATA + 64); + + for instruction in [0xc200, 0xb400] { + for (address, error) in [ + (ROM_START, MemoryError::ReadOnly(ROM_START)), + (0x0e00_0000, MemoryError::Unmapped(0x0e00_0000)), + ] { + let (mut cpu, mut memory) = setup(instruction, CODE); + let register = if instruction == 0xb400 { 13 } else { 2 }; + cpu.registers[register] = address + if register == 13 { 64 } else { 0 }; + memory.write32(TIMER_BASE, 0x0080_0000).unwrap(); + let before = cpu.clone(); + let mut machine = Machine::new(cpu, memory); + assert_eq!( + machine.step(), + Err(MachineError::Cpu(CpuError::Memory(error))) + ); + assert_eq!(machine.cpu(), &before); + assert_eq!(machine.cycles(), 0); + assert_eq!(machine.last_timing(), StepTiming::default()); + assert_eq!(machine.memory().read16(TIMER_BASE).unwrap(), 0); + } + } +} + +#[test] +fn push_lr_is_not_an_empty_list_and_keeps_the_original_link_value() { + let (mut cpu, mut memory) = setup(0xb500, CODE); // PUSH {lr} + cpu.registers[13] = DATA + 4; + cpu.registers[14] = 0x1234_5679; + cpu.step(&mut memory).unwrap(); + assert_eq!(cpu.registers[13], DATA); + assert_eq!(memory.read32(DATA).unwrap(), 0x1234_5679); + assert_eq!(cpu.pc(), CODE + 2); +} diff --git a/crates/gba-core/src/cpu/thumb_tests.rs b/crates/gba-core/src/cpu/thumb_tests.rs index 1d53e9b..3bf52ba 100644 --- a/crates/gba-core/src/cpu/thumb_tests.rs +++ b/crates/gba-core/src/cpu/thumb_tests.rs @@ -560,7 +560,7 @@ fn empty_thumb_multiple_lists_transfer_pc_with_sixty_four_byte_writeback() { cpu.step(&mut memory).unwrap(); assert_eq!(cpu.registers[0], RAM + 64); if instruction == 0xc000 { - assert_eq!(memory.read32(RAM).unwrap(), ROM_START + 4); + assert_eq!(memory.read32(RAM).unwrap(), ROM_START + 6); assert_eq!(cpu.pc(), ROM_START + 2); } else { assert_eq!(cpu.pc(), ROM_START + 10); @@ -576,10 +576,10 @@ fn empty_push_pop_follow_arm7_block_transfer_rules() { cpu.registers[13] = RAM + 0x100; cpu.step(&mut memory).unwrap(); assert_eq!(cpu.registers[13], RAM + 0xc0); - assert_eq!(memory.read32(RAM + 0xc0).unwrap(), ROM_START + 4); + assert_eq!(memory.read32(RAM + 0xc0).unwrap(), ROM_START + 6); cpu.step(&mut memory).unwrap(); assert_eq!(cpu.registers[13], RAM + 0x100); - assert_eq!(cpu.pc(), ROM_START + 4); + assert_eq!(cpu.pc(), ROM_START + 6); } #[test] diff --git a/crates/gba-core/src/cpu/transfer.rs b/crates/gba-core/src/cpu/transfer.rs index 5b05ae3..a15a603 100644 --- a/crates/gba-core/src/cpu/transfer.rs +++ b/crates/gba-core/src/cpu/transfer.rs @@ -85,10 +85,12 @@ impl Cpu { } else if user_bank { self.user_register(register) } else { + // Stores use one instruction beyond the ordinary operand PC. + // Thumb can store PC here only through an empty register list. let pc_offset = if self.instruction_set == InstructionSet::Arm { 12 } else { - 4 + 6 }; self.operand_register(register, pc_offset) }; diff --git a/docs/architecture.md b/docs/architecture.md index 921a2b4..f057f86 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -30,6 +30,7 @@ The repository is a Cargo workspace with three crates: | `crates/gba-core/src/cpu/compare_psr_tests.rs` | Original Rd=15 test/compare status restoration, flags, bank, and timing regressions | | `crates/gba-core/src/cpu/status_tests.rs`, `crates/gba-core/src/cpu/exception_tests.rs` | Status, banking, exception, and return tests | | `crates/gba-core/src/cpu/thumb_tests.rs` | Thumb instructions, state switching, and decoder checks | +| `crates/gba-core/src/cpu/thumb_empty_tests.rs` | Original empty-list stored-PC regressions for banks, alignment, writeback, timing, and diagnostics | | `crates/gba-core/src/cpu/transfer_tests.rs` | Stack, addressing, overlap, empty-list, and swap tests | | `crates/gba-core/src/cpu/tests.rs` | Condition truth tables and immediate arithmetic tests | | `crates/gba-core/src/cpu/instruction_tests.rs` | Register operations, transfers, branches, and edge cases | diff --git a/docs/hardware/cpu.md b/docs/hardware/cpu.md index 4051d6f..4ec4ccc 100644 --- a/docs/hardware/cpu.md +++ b/docs/hardware/cpu.md @@ -38,6 +38,7 @@ Arithmetic operations compute carry and overflow from the arithmetic result, not The CPU stores the next instruction address in r15. In ARM state, ordinary operand reads add eight bytes. Register-specified shifts use a twelve-byte offset. Storing r15 in ARM state also uses a twelve-byte offset. +Thumb empty-register-list stores use PC plus six; they must not use the ordinary Thumb operand offset. Thumb operand reads use PC plus four. Literal loads and PC-relative addresses additionally align down to a word. `BX` selects the instruction set; ordinary PC writes keep the current instruction set. On ARM7, Thumb `POP {pc}` remains in Thumb even if the loaded address has bit zero clear. @@ -64,6 +65,13 @@ These alias rules implement ARM7 compatibility behavior, not a recommendation fo Block transfers assign the lowest-numbered register to the lowest memory address. They align addresses down without rotating loaded words, and preserve low base bits during writeback. On ARM7, an empty register list transfers PC but uses a 64-byte span for addressing and writeback. +Thumb `STMIA Rb!, {}` stores executing PC+6 at the word-aligned base, then adds 64 to the unaligned base. +The shared empty `PUSH {}` path also stores PC+6, retaining its existing full-descending addressing and 64-byte stack adjustment. +The stored PC keeps bit 1; neither instruction word-aligns this value. Both stores advance execution by two bytes and preserve flags. +Empty Thumb loads still align the loaded PC to a halfword and remain in Thumb state. +`PUSH {lr}` is not an empty list and stores the original link register with a four-byte stack adjustment. +The PC+6 correction agrees with public Thumb test 229 and mGBA's shared empty-list store path. +These forms retain nominal single-word access costs and the existing atomic-error policy; exact bus timing remains unverified. `LDM` suppresses writeback when the base register is in the list. With writeback enabled, `STM` stores the old base only when that register is first in the list. Otherwise, `STM` stores the updated base. diff --git a/docs/public-arm-tests.md b/docs/public-arm-tests.md index 3f4eacb..764284e 100644 --- a/docs/public-arm-tests.md +++ b/docs/public-arm-tests.md @@ -110,7 +110,8 @@ Test 362 checks shifted addressing, writeback, and carry, but does not check the Original regressions separately check the returned word, byte lanes, rotation, sign extension, timing, and retained diagnostics. See the bounded [ARM open-bus implementation](hardware/cpu.md#arm-unused-memory-data-reads). This is a passing result for this pinned ARM ROM, not proof of complete instruction, memory, or timing compatibility. -No public Thumb, timing, memory, BIOS, graphics, or unsafe suite has been claimed as passing. +The [pinned public Thumb ROM](public-thumb-tests.md) also passes its verified checkpoint. +No public timing, memory, BIOS, graphics, or unsafe suite has been claimed as passing. These results do not establish Pokémon Emerald compatibility. ## References diff --git a/docs/public-thumb-tests.md b/docs/public-thumb-tests.md index 7d532cd..a375227 100644 --- a/docs/public-thumb-tests.md +++ b/docs/public-thumb-tests.md @@ -1,7 +1,7 @@ # Pinned public Thumb test The headless runner executes the unmodified Thumb ROM from [jsmolka/gba-tests](https://github.com/jsmolka/gba-tests). -It currently reaches its result checkpoint but fails test 229 on Darwin arm64 in debug and release builds. +It now passes its result checkpoint on Darwin arm64 in debug and release builds. The initial test 229 failure is retained below. This is an independent compatibility baseline, not complete ARM7TDMI conformance or a hardware measurement. ## Prepare and run @@ -67,8 +67,20 @@ Initial integration, before fixing empty-list Thumb stores: - The stored PC value is `0x080008be`, but the following PC read expects `0x080008c0`. - The public ARM ROM still passes its unchanged checkpoint. -The next CPU task is to verify the empty-list store PC offset and correct it with original regressions. -Do not change the pinned ROM or weaken the result assertion to pass this test. +After correcting the empty-list store PC offset: + +- Thumb empty-list stores use executing PC+6 instead of PC+4. ARM stores retain PC+12. +- Test 229 and the later memory tests complete without taking their failure paths. +- The runner reaches `0x08000934` after 807 successful steps and 4,170 nominal cycles. +- The CPU is in System/ARM state with CPSR `0x0000001f` and r7 = 0. +- Debug and release builds exit zero with `reason: checkpoint`, no error, and the result assertion passing. +- Both public ROMs pass with their original bytes, checkpoint addresses, budgets, and assertions unchanged. + +Original regressions reproduce the stored-PC mismatch without public ROM bytes. +They cover all low base registers, banked stack pointers, code/data alignment, ROM windows, timing, and atomic diagnostics. +Existing empty-list expectations that incorrectly required PC+4 were corrected; empty loads and ARM stores retain their behavior. +See [CPU transfer semantics](hardware/cpu.md#instruction-demo-and-execution-rules). +This proves only the pinned suites' checkpoints and assertions, not complete CPU, bus, or timing compatibility. No public timing, memory, BIOS, graphics, or unsafe suite is claimed as passing. These results do not establish Pokémon Emerald compatibility. diff --git a/docs/references.md b/docs/references.md index 5ab8b3b..eabd9f5 100644 --- a/docs/references.md +++ b/docs/references.md @@ -78,7 +78,8 @@ Hardware references used for the core: - [GBATEK Thumb register operations](https://problemkaputt.de/gbatek-thumb-opcodes-register-operations-alu-bx.htm). - [GBATEK Thumb loads/stores](https://problemkaputt.de/gbatek-thumb-opcodes-memory-load-store-ldr-str.htm). - [GBATEK Thumb address calculation](https://problemkaputt.de/gbatek-thumb-opcodes-memory-addressing-add-pc-sp.htm). -- [GBATEK Thumb stack and multiple transfers](https://problemkaputt.de/gbatek-thumb-opcodes-memory-multiple-load-store-push-pop-and-ldm-stm.htm). +- [GBATEK Thumb stack and multiple transfers](https://problemkaputt.de/gbatek-thumb-opcodes-memory-multiple-load-store-push-pop-and-ldm-stm.htm), for empty-list PC transfers and 64-byte writeback. +- [mGBA Thumb transfer implementation](https://github.com/mgba-emu/mgba/blob/master/src/arm/isa-thumb.c) and [shared memory transfers](https://github.com/mgba-emu/mgba/blob/master/src/gba/memory.c), reviewed for empty STM/PUSH routing and the extra Thumb instruction width on the stored pipeline PC. This supports executing PC+6, rather than an ordinary PC+4 operand read. - [GBATEK Thumb jumps and calls](https://problemkaputt.de/gbatek-thumb-opcodes-jumps-and-calls.htm). Use [GBATEK](https://problemkaputt.de/gbatek.htm) and ARM7TDMI documentation for further hardware work. diff --git a/docs/rom-tests.md b/docs/rom-tests.md index 9e272a3..653ad29 100644 --- a/docs/rom-tests.md +++ b/docs/rom-tests.md @@ -5,7 +5,7 @@ Each case has a machine-step budget, a completion address, and explicit assertio A timeout never counts as a pass. Existing `--rom ... --steps ...` remains a diagnostic mode, not a test verdict. Original programs cover the runner itself. Pinned public [ARM](public-arm-tests.md) and [Thumb](public-thumb-tests.md) ROMs provide independent compatibility results. -The ARM ROM passes its checkpoint. The Thumb ROM currently reports test 229 as failed. +Both pinned ROMs pass their checkpoints on Darwin arm64 in debug and release builds. Passing the runner's unit tests does not mean an external suite passes. A passing suite establishes only its specified checkpoints and assertions. It does not establish Emerald compatibility. @@ -133,8 +133,8 @@ They do not hash ROM contents; record source revisions and file checksums alongs The first public integration is [jsmolka/gba-tests ARM](public-arm-tests.md), with a verified result-register checkpoint. The pinned ARM ROM now passes this checkpoint on Darwin arm64. Historical failures remain documented. -The [pinned Thumb ROM](public-thumb-tests.md) also has a verified checkpoint, but currently fails its r7 assertion at test 229. -Verify and correct the empty-list store PC value before claiming that suite passes. +The [pinned Thumb ROM](public-thumb-tests.md) also passes its verified r7 checkpoint after the empty-list store correction. +Next, integrate a pinned public memory suite with a verified result protocol. For further public ARM7TDMI/GBA tests, inspect their source, license, entry assumptions, and result protocol. Pin source revisions and build instructions before comparing emulator changes. Adapt suites to verified completion addresses and result locations; do not guess them or treat timeouts as passes. diff --git a/docs/status.md b/docs/status.md index 593b850..a65c62b 100644 --- a/docs/status.md +++ b/docs/status.md @@ -68,7 +68,7 @@ Update this file when a feature lands or a limit is removed. - Optional ROM-window frame limits and an original input-test ROM generator. - Headless JSON ROM suites with per-case checkpoints, register/CPSR/memory assertions, bounded budgets, and structured failure reports. - Hash-pinned preparation of public `jsmolka/gba-tests` ARM and Thumb ROMs, with verified result-register checkpoints. -- The public ARM ROM passes on Darwin arm64; the Thumb ROM has an explicit failing baseline at test 229. +- The pinned public ARM and Thumb ROMs pass their checkpoints on Darwin arm64 in debug and release builds. - ARM test/compare Rd=15 status restoration, without a PC result write or nominal refill. - ARM unused-memory data reads return a PC+8 word snapshot, with normal byte lanes and load rotation. - Little-endian byte, halfword, and word reads/writes. @@ -81,7 +81,7 @@ Thumb execution includes: - High-register operations and `BX`. - Word, byte, halfword, and signed loads/stores with register or immediate offsets. - PC-relative literal loads, PC/SP-relative address calculation, and stack-pointer adjustment. -- `PUSH`/`POP` and multiple-register loads/stores. +- `PUSH`/`POP` and multiple-register loads/stores, including PC+6 stores for empty register lists. - All 14 conditional branches, unconditional branches, and both halves of `BL`. ## Deliberate limits @@ -117,7 +117,8 @@ Audio, saves, and external BIOS loading remain unavailable. ROM loading does not See [cartridge loading](hardware/cartridge.md) for file limits, step budgets, and exit status. The [ROM suite runner](rom-tests.md) has original regressions and a [pinned public ARM baseline](public-arm-tests.md). The pinned public ARM ROM passes its result checkpoint in debug and release builds. -The [pinned public Thumb ROM](public-thumb-tests.md) reaches its checkpoint but reports test 229 as failed. Other public suites remain unverified. +The [pinned public Thumb ROM](public-thumb-tests.md) also passes after correcting its empty-list store PC value. +Other public suites remain unverified. These results do not establish full CPU compatibility. The runner requires known completion addresses and has no debug-port protocol, scripted input, or rendered-image assertions. ARM data reads from unused address ranges use a bounded [PC+8 open-bus snapshot](hardware/cpu.md#arm-unused-memory-data-reads). @@ -130,7 +131,7 @@ Writes to cartridge addresses return errors instead of modeling cartridge hardwa ## Next steps -1. Verify and fix the empty-list Thumb store PC value exposed by public test 229, then rerun both pinned suites. +1. Add a pinned public memory suite with a verified result protocol, then address its first compatibility failure. Extend open-bus support with verified Thumb region/alignment rules and bus history, rather than a blanket unmapped-read fallback. 2. Refine nominal timing with a fetch pipeline, Game Pak prefetch, per-access device updates, and verified timer/IRQ delays. 3. Validate keypad retrigger behavior with hardware tests and add remaining device registers; extend BIOS reset coverage as sound/serial support becomes available. diff --git a/docs/testing.md b/docs/testing.md index 9af991a..5fbeb3c 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -65,7 +65,11 @@ They verify loaded-value precedence, preserved flags, unchanged stores, nominal ARM open-bus regressions cover unused-range boundaries, PC+8 source regions, byte lanes, signed/unaligned loads, writeback, and block loads. They check branch-target resampling, missing lookahead, skipped conditions, nominal timing, device progress, and retained host/fetch/Thumb/DMA diagnostics. The pinned public ARM ROM now reaches its completion checkpoint with r12 = 0 in debug and release builds. -This does not establish public Thumb, timing, graphics, BIOS, or commercial-game compatibility. +The pinned public Thumb ROM also reaches its ARM-state checkpoint with r7 = 0 in both builds. +Empty-list Thumb regressions cover PC+6, all low bases and processor modes, banked stacks, both code alignments, and unaligned data. +They check one-word writes, 64-byte writeback, ROM windows, the following PC operand, nominal timing, and atomic errors. +Existing ARM empty-list and Thumb empty-load tests continue to pass. `PUSH {lr}` retains its normal link value. +These results do not establish public timing, memory, graphics, BIOS, or commercial-game compatibility. ROM-runner tests use temporary files containing original instructions, without Nintendo assets or a desktop session. They check file-size boundaries, bounded reads, short reads, I/O errors, paths, symlinks, and read-only loading.