diff --git a/README.md b/README.md index af8b6c6..b78c122 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,7 @@ start Main | [`sprites_and_palettes.ne`](examples/sprites_and_palettes.ne) | Inline CHR data, scroll, type casting | | [`mmc1_banked.ne`](examples/mmc1_banked.ne) | MMC1 mapper, bank declarations, multiply | | [`uxrom_user_banked.ne`](examples/uxrom_user_banked.ne) | UxROM mapper with a `bank Foo { fun ... }` block — first example to put real user code in a switchable bank, called via a generated cross-bank trampoline | +| [`uxrom_banked_to_banked.ne`](examples/uxrom_banked_to_banked.ne) | UxROM with two `bank Foo { fun ... }` blocks — exercises a banked→banked call (`step` in `Logic` calls `clamp` in `Helpers`) routed through the same trampoline that handles fixed→banked | | [`palette_and_background.ne`](examples/palette_and_background.ne) | Palette and background declarations, reset-time load, vblank-safe `set_palette` / `load_background` swaps | | [`friendly_assets.ne`](examples/friendly_assets.ne) | **Pleasant asset syntax** — named NES colours, grouped `bg0..sp3` palettes with `universal:`, ASCII pixel-art sprites, `legend { } + map:` tilemaps, `palette_map:` attribute grids, scalar sfx `pitch:`, note-name music with `tempo:` | | [`structs_enums_for.ne`](examples/structs_enums_for.ne) | Structs, enums, `for` loops, struct literals | diff --git a/examples/README.md b/examples/README.md index 53a0ea3..6e2fa5a 100644 --- a/examples/README.md +++ b/examples/README.md @@ -27,6 +27,7 @@ Open any `.nes` file in an NES emulator ([Mesen](https://www.mesen.ca/), [FCEUX] | `sprites_and_palettes.ne` | sprites, scroll, cast | Inline CHR data, PPU scroll writes, type casting | | `mmc1_banked.ne` | MMC1, banks, multiply | Banked mapper with software multiply | | `uxrom_user_banked.ne` | UxROM, `bank Foo { fun ... }`, cross-bank trampoline | First example to put real user code inside a switchable bank. The animation step lives in `bank Extras` and is invoked from the fixed-bank state handler via a generated `__tramp_step_animation` stub that selects bank 0, JSRs the body, then restores the fixed bank before returning. | +| `uxrom_banked_to_banked.ne` | UxROM, banked → banked cross-bank call | Two `bank Foo { fun ... }` blocks: `step` lives in bank Logic and calls `clamp` in bank Helpers. The trampoline uses `ZP_BANK_CURRENT + PHA/PLA` to save and restore the caller's bank, so the same per-callee stub works whether the caller is in the fixed bank or another switchable bank. | | `palette_and_background.ne` | palette, background, set_palette, load_background | Reset-time initial load plus vblank-safe runtime swaps | | `friendly_assets.ne` | named colours, grouped palette, pixel art, tilemap+legend, palette_map, scalar sfx pitch, note-name music | Exercises every "friendlier" asset syntax at once — the `palette` uses `bg0..sp3` + a shared `universal:`, the sprite is authored as ASCII pixel art, the background uses a `legend { ... } + map:` tilemap with a `palette_map:` for attributes, the sfx uses a scalar `pitch:` + `envelope:` alias, and the music uses note names (`C4, E4 40, rest 10`) with a `tempo:` default. | | `noise_triangle_sfx.ne` | `channel: noise`, `channel: triangle` on `sfx` blocks | Demonstrates the noise and triangle sfx channels. Declares one noise burst and one triangle bass note, plays each on a timer so the emulator harness captures both the pixel output and the APU state. | diff --git a/examples/mmc1_banked.nes b/examples/mmc1_banked.nes index 3d28138..e5e1109 100644 Binary files a/examples/mmc1_banked.nes and b/examples/mmc1_banked.nes differ diff --git a/examples/mmc3_per_state_split.nes b/examples/mmc3_per_state_split.nes index 857af30..99754ac 100644 Binary files a/examples/mmc3_per_state_split.nes and b/examples/mmc3_per_state_split.nes differ diff --git a/examples/scanline_split.nes b/examples/scanline_split.nes index 5f95708..93ba691 100644 Binary files a/examples/scanline_split.nes and b/examples/scanline_split.nes differ diff --git a/examples/uxrom_banked.nes b/examples/uxrom_banked.nes index 7c9e513..3b3aac3 100644 Binary files a/examples/uxrom_banked.nes and b/examples/uxrom_banked.nes differ diff --git a/examples/uxrom_banked_to_banked.ne b/examples/uxrom_banked_to_banked.ne new file mode 100644 index 0000000..a4105fb --- /dev/null +++ b/examples/uxrom_banked_to_banked.ne @@ -0,0 +1,74 @@ +// UxROM Banked-to-Banked — first NEScript example to exercise a +// switchable-bank function calling *another* switchable-bank function. +// +// The previous user-banked example (`uxrom_user_banked.ne`) only put +// fixed → banked calls through the trampoline path; here `bank Logic` +// holds `step()` and `bank Helpers` holds `clamp()`, and `step` calls +// `clamp` once per frame. The codegen emits `JSR __tramp_clamp` from +// inside bank Logic, which lands in the fixed-bank trampoline that +// saves the current bank (Logic), switches to Helpers, runs the body, +// then restores Logic on the way out — see runtime/gen_bank_trampoline +// for the PHA/PLA implementation. +// +// The harness captures frame 180 somewhere along the sweep, so any +// regression in the trampoline's save/restore would either leave the +// wrong bank mapped at $8000 (subsequent sprite reads would corrupt) +// or crash before the OAM update happened. +// +// Build: cargo run -- build examples/uxrom_banked_to_banked.ne + +game "UxROM Banked to Banked" { + mapper: UxROM + mirroring: horizontal +} + +// Globals live in the fixed bank's RAM and are reachable from any +// bank via direct zero-page / absolute addressing — bank switching +// only affects the $8000-$BFFF code window. +var px: u8 = 80 +var dir: u8 = 0 // 0 = sweep right, 1 = sweep left + +bank Logic { + // The "main" banked function. Lives in bank Logic and calls + // into bank Helpers via the fixed-bank trampoline emitted + // by the linker. + fun step() { + if dir == 0 { + px = px + 1 + } else { + px = px - 1 + } + // Cross-bank call into Helpers. The codegen sees a + // current_bank of "Logic" and a callee bank of "Helpers" + // and emits `JSR __tramp_clamp` — the trampoline lives + // in the fixed bank, saves the caller's bank + // (ZP_BANK_CURRENT == Logic), switches to Helpers, runs + // the body, then restores Logic before returning here. + clamp() + } +} + +bank Helpers { + // Bounce the sprite between two pixel rails. Self-contained + // — only reads/writes the global zero-page slots, no calls + // back out of the bank, so the trampoline never has to + // recursively unwind a third level. + fun clamp() { + if px == 176 { + dir = 1 + } + if px == 80 { + dir = 0 + } + } +} + +on frame { + // Single fixed → banked trampoline call. Inside, `step` does a + // banked → banked call into `clamp` — that second hop is the + // path the new trampoline implementation enables. + step() + draw Smiley at: (px, 112) +} + +start Main diff --git a/examples/uxrom_banked_to_banked.nes b/examples/uxrom_banked_to_banked.nes new file mode 100644 index 0000000..b4ebeec Binary files /dev/null and b/examples/uxrom_banked_to_banked.nes differ diff --git a/examples/uxrom_user_banked.nes b/examples/uxrom_user_banked.nes index 94db0fa..552aa1a 100644 Binary files a/examples/uxrom_user_banked.nes and b/examples/uxrom_user_banked.nes differ diff --git a/src/codegen/ir_codegen.rs b/src/codegen/ir_codegen.rs index 44adfe6..5fb950a 100644 --- a/src/codegen/ir_codegen.rs +++ b/src/codegen/ir_codegen.rs @@ -273,6 +273,23 @@ impl<'a> IrCodeGen<'a> { } } + /// Suffix used by the codegen's local-label generators (e.g. + /// `__ir_cmp_e_`, `__ir_wait_`). For fixed-bank + /// code this is just the current `instructions.len()`, which + /// matches the pre-banked-banked behaviour byte-for-byte. + /// For *banked* code we additionally prefix the bank name so + /// labels can't collide across two switchable banks — the + /// linker's discovery pass panics on duplicate labels across + /// switchable banks, which used to make banked → banked + /// codegen impossible to test even after the trampoline path + /// was fixed. + fn local_label_suffix(&self) -> String { + match &self.current_bank { + None => format!("{}", self.instructions.len()), + Some(bank) => format!("{bank}_{}", self.instructions.len()), + } + } + /// Per-banked-function instruction streams produced by the most /// recent [`Self::generate`] call. The map is keyed by bank name /// (matching the program's `bank Foo { ... }` declarations) and @@ -470,7 +487,7 @@ impl<'a> IrCodeGen<'a> { // that the short-branch fixup would panic at link time. // BCC-over-JMP keeps the hot path at two branches (well // under 8 cycles) and the failure path at a 3-byte JMP. - let skip_label = format!("__ir_bc_ok_{}", self.instructions.len()); + let skip_label = format!("__ir_bc_ok_{}", self.local_label_suffix()); self.emit(CMP, AM::Immediate(size_u8)); self.emit(BCC, AM::LabelRelative(skip_label.clone())); self.emit(JMP, AM::Label("__debug_halt".to_string())); @@ -489,7 +506,7 @@ impl<'a> IrCodeGen<'a> { amt: IrTemp, shift_op: crate::asm::Opcode, ) { - let suffix = self.instructions.len(); + let suffix = self.local_label_suffix(); let loop_label = format!("__ir_shift_loop_{suffix}"); let done_label = format!("__ir_shift_done_{suffix}"); let amt_addr = self.temp_addr(amt); @@ -974,21 +991,30 @@ impl<'a> IrCodeGen<'a> { self.load_temp(*arg); self.emit(STA, AM::ZeroPage(0x04 + i as u8)); } - // Pick the right JSR target. Three cases: - // 1. Callee is in the fixed bank (most common): - // JSR `__ir_fn_` — the original behaviour. - // 2. Callee is in a switchable bank and the caller - // is in the fixed bank: JSR `__tramp_`, - // the linker-emitted trampoline that switches - // banks, calls the body, then switches back. - // 3. Caller and callee live in the same switchable - // bank: direct JSR to `__ir_fn_` works - // because both labels exist in the bank's own - // assembler pass. + // Pick the right JSR target. Four cases: + // 1. Caller and callee are both in the fixed bank + // (most common): JSR `__ir_fn_` directly. + // 2. Caller is in the fixed bank, callee is in a + // switchable bank: JSR `__tramp_`, the + // linker-emitted trampoline that swaps to the + // target bank, runs the callee, then restores + // the caller's bank. + // 3. Caller and callee live in the *same* + // switchable bank: direct JSR to `__ir_fn_` + // — both labels exist in the bank's own + // assembler pass, so the link resolves locally + // without going through the fixed bank. + // 4. Caller and callee live in *different* + // switchable banks: same trampoline as case 2. + // The trampoline reads `ZP_BANK_CURRENT` to + // figure out which bank to restore on the way + // out, so it doesn't need to know the caller's + // bank at link time. // - // Cross-bank calls between two different switchable - // banks aren't supported in the first pass — the - // codegen panics rather than silently miscompiling. + // Banked → fixed bank calls (case 5) work without a + // trampoline because the fixed bank is always mapped + // at $C000-$FFFF — a direct JSR into the fixed bank + // doesn't need any bank-switching. let callee_bank = self.function_banks.get(name).cloned(); let label = match (&self.current_bank, &callee_bank) { (None, None) => format!("__ir_fn_{name}"), @@ -996,13 +1022,13 @@ impl<'a> IrCodeGen<'a> { (Some(from_bank), Some(to_bank)) if from_bank == to_bank => { format!("__ir_fn_{name}") } - (Some(from_bank), Some(to_bank)) => { - panic!( - "cross-bank call from bank '{from_bank}' to '{to_bank}' \ - is not supported (function '{name}'); only fixed-bank \ - callers can invoke banked functions in the v1 \ - user-banked codegen" - ); + (Some(_), Some(_)) => { + // Banked → banked cross-bank call. The + // fixed-bank trampoline saves the caller's + // current bank, switches to the callee's + // bank, calls, then restores the caller's + // bank — same path as fixed → banked. + format!("__tramp_{name}") } (Some(_), None) => { // Banked function calls a fixed-bank function. @@ -1098,7 +1124,7 @@ impl<'a> IrCodeGen<'a> { // previous frame overrun" sticky bit so user code // sees a fresh value next NMI. The cumulative // counter at $07FF is intentionally left alone. - let wait_label = format!("__ir_wait_{}", self.instructions.len()); + let wait_label = format!("__ir_wait_{}", self.local_label_suffix()); self.emit_label(&wait_label); self.emit(LDA, AM::ZeroPage(ZP_FRAME_FLAG)); self.emit(BEQ, AM::LabelRelative(wait_label)); @@ -1148,7 +1174,7 @@ impl<'a> IrCodeGen<'a> { if self.debug_mode { // Load cond; if nonzero (true) skip; else halt self.load_temp(*cond); - let pass_label = format!("__ir_assert_pass_{}", self.instructions.len()); + let pass_label = format!("__ir_assert_pass_{}", self.local_label_suffix()); self.emit(BNE, AM::LabelRelative(pass_label.clone())); // Assertion failed: write marker to debug port and BRK self.emit(LDA, AM::Immediate(0xFF)); @@ -1736,10 +1762,11 @@ impl<'a> IrCodeGen<'a> { b_hi: IrTemp, kind: Cmp16Kind, ) { - let true_label = format!("__ir_cmp16_t_{}", self.instructions.len()); - let false_label = format!("__ir_cmp16_f_{}", self.instructions.len()); - let end_label = format!("__ir_cmp16_e_{}", self.instructions.len()); - let lo_label = format!("__ir_cmp16_lo_{}", self.instructions.len()); + let suffix = self.local_label_suffix(); + let true_label = format!("__ir_cmp16_t_{suffix}"); + let false_label = format!("__ir_cmp16_f_{suffix}"); + let end_label = format!("__ir_cmp16_e_{suffix}"); + let lo_label = format!("__ir_cmp16_lo_{suffix}"); // Compare high bytes. self.load_temp(a_hi); @@ -1829,8 +1856,9 @@ impl<'a> IrCodeGen<'a> { let b_addr = self.temp_addr(b); self.emit(CMP, AM::ZeroPage(b_addr)); - let true_label = format!("__ir_cmp_t_{}", self.instructions.len()); - let end_label = format!("__ir_cmp_e_{}", self.instructions.len()); + let suffix = self.local_label_suffix(); + let true_label = format!("__ir_cmp_t_{suffix}"); + let end_label = format!("__ir_cmp_e_{suffix}"); match kind { CmpKind::Eq => self.emit(BEQ, AM::LabelRelative(true_label.clone())), @@ -3300,6 +3328,121 @@ mod more_tests { ); } + #[test] + fn ir_codegen_banked_to_banked_call_emits_trampoline_jsr() { + // A banked function that calls another function in a + // *different* switchable bank should JSR `__tramp_`, + // not `__ir_fn_`. The codegen previously panicked + // for this case; the trampoline now save/restores the + // caller's bank so a single per-callee stub works for any + // caller bank. The JSR itself lands inside the caller + // bank's banked instruction stream — fixed-bank code is + // unaffected. + let (prog, _) = parser::parse( + r#" + game "T" { mapper: UxROM } + var x: u8 = 0 + bank Logic { + fun step() { helper() } + } + bank Helpers { + fun helper() { x = x + 1 } + } + on frame { step() } + start Main + "#, + ); + let prog = prog.unwrap(); + let analysis = analyzer::analyze(&prog); + let ir_program = ir::lower(&prog, &analysis); + let mut codegen = IrCodeGen::new(&analysis.var_allocations, &ir_program); + codegen.generate(&ir_program); + let banked = codegen.banked_streams(); + let logic_stream = banked + .get("Logic") + .expect("expected Logic bank stream from codegen"); + let helper_jsr = logic_stream + .iter() + .any(|i| i.opcode == JSR && matches!(&i.mode, AM::Label(l) if l == "__tramp_helper")); + assert!( + helper_jsr, + "Logic bank's `step` body should JSR __tramp_helper for the banked → banked call" + ); + // And critically, the same stream should NOT contain a + // direct `JSR __ir_fn_helper` — that would jump straight + // into a $8000-window address that isn't currently mapped. + let direct_jsr = logic_stream + .iter() + .any(|i| i.opcode == JSR && matches!(&i.mode, AM::Label(l) if l == "__ir_fn_helper")); + assert!( + !direct_jsr, + "banked → banked codegen must go through the trampoline, not __ir_fn_helper" + ); + } + + #[test] + fn ir_codegen_local_label_suffix_is_bank_namespaced() { + // When two banked functions in *different* banks both emit + // a local label like `__ir_cmp_e_`, the suffix has to + // include the bank name so the linker's discovery pass + // (which checks for cross-bank label collisions) doesn't + // panic on the second occurrence. Without the namespacing + // step, this exact program used to fail at link time with + // `duplicate label '__ir_cmp_e_8' across switchable banks`. + let (prog, _) = parser::parse( + r#" + game "T" { mapper: UxROM } + var x: u8 = 0 + bank A { + fun a_fn() { if x == 0 { x = 1 } } + } + bank B { + fun b_fn() { if x == 0 { x = 2 } } + } + on frame { a_fn() b_fn() } + start Main + "#, + ); + let prog = prog.unwrap(); + let analysis = analyzer::analyze(&prog); + let ir_program = ir::lower(&prog, &analysis); + let mut codegen = IrCodeGen::new(&analysis.var_allocations, &ir_program); + codegen.generate(&ir_program); + let banked = codegen.banked_streams(); + let a_labels: Vec<_> = banked + .get("A") + .expect("A stream") + .iter() + .filter_map(|i| match &i.mode { + AM::Label(l) if l.contains("__ir_cmp") => Some(l.clone()), + _ => None, + }) + .collect(); + let b_labels: Vec<_> = banked + .get("B") + .expect("B stream") + .iter() + .filter_map(|i| match &i.mode { + AM::Label(l) if l.contains("__ir_cmp") => Some(l.clone()), + _ => None, + }) + .collect(); + assert!( + !a_labels.is_empty(), + "bank A should emit at least one cmp label" + ); + assert!( + !b_labels.is_empty(), + "bank B should emit at least one cmp label" + ); + for a in &a_labels { + assert!( + !b_labels.contains(a), + "bank A label '{a}' collides with one in bank B" + ); + } + } + #[test] fn ir_codegen_source_map_opt_in_emits_src_labels() { // With `with_source_map(true)` the codegen should emit diff --git a/src/linker/mod.rs b/src/linker/mod.rs index dab692e..113cbbf 100644 --- a/src/linker/mod.rs +++ b/src/linker/mod.rs @@ -341,7 +341,6 @@ impl Linker { // maps to $C000-$FFFF and one of the switchable banks maps // to $8000-$BFFF. let total_banks = switchable_banks.len() + 1; - let fixed_bank_index = total_banks - 1; // Discovery pass: assemble each switchable bank that has // its own instruction stream so we know what labels live @@ -447,8 +446,6 @@ impl Linker { // which switchable bank is currently mapped at $8000. if self.mapper != Mapper::NROM { all_instructions.extend(runtime::gen_bank_select(self.mapper)); - #[allow(clippy::cast_possible_truncation)] - let fixed_bank_num = fixed_bank_index as u8; for (i, bank) in switchable_banks.iter().enumerate() { if bank.trampolines.is_empty() { continue; @@ -460,7 +457,6 @@ impl Linker { &tramp.tramp_label, &tramp.entry_label, bank_num, - fixed_bank_num, )); } } diff --git a/src/runtime/mod.rs b/src/runtime/mod.rs index 7e66abb..5911d3a 100644 --- a/src/runtime/mod.rs +++ b/src/runtime/mod.rs @@ -1332,12 +1332,28 @@ pub fn gen_mapper_init( mirroring: Mirroring, total_prg_banks: usize, ) -> Vec { - match mapper { + let mut out = match mapper { Mapper::NROM => Vec::new(), Mapper::MMC1 => gen_mmc1_init(mirroring), Mapper::UxROM => gen_uxrom_init(total_prg_banks), Mapper::MMC3 => gen_mmc3_init(mirroring), + }; + // Initialize ZP_BANK_CURRENT to the fixed bank index for any + // banked mapper. The trampoline emitted by + // `gen_bank_trampoline` reads this slot to decide which bank + // to restore after a cross-bank call, so it has to be a + // sensible value from the very first call. Without this the + // RAM-clear leaves it at $00, which would put bank 0 at + // $8000 instead of the fixed bank after a fixed-bank caller's + // first cross-bank call — a behavior change vs. the pre- + // banked-banked codegen that some examples rely on. + if mapper != Mapper::NROM && total_prg_banks > 0 { + #[allow(clippy::cast_possible_truncation)] + let fixed_bank_index = (total_prg_banks - 1) as u8; + out.push(Instruction::new(LDA, AM::Immediate(fixed_bank_index))); + out.push(Instruction::new(STA, AM::ZeroPage(ZP_BANK_CURRENT))); } + out } /// MMC1 reset: pulse the reset bit, then write the control register. @@ -1517,13 +1533,28 @@ pub fn gen_bank_select(mapper: Mapper) -> Vec { } /// Generate a cross-bank trampoline stub. Placed in the fixed bank -/// and called by user code (also in the fixed bank) via -/// `JSR `. Behavior: +/// and called by *any* user code via `JSR ` regardless +/// of which bank the caller currently lives in. Behavior: /// -/// 1. Load the target bank number into A, JSR `__bank_select`. -/// 2. JSR the user-supplied entry label inside the target bank. -/// 3. Load the fixed bank number, JSR `__bank_select` to restore. -/// 4. RTS. +/// 1. Read [`ZP_BANK_CURRENT`] into A, push it on the hardware +/// stack — that's the bank we'll need to switch back to. +/// 2. Load the target bank number into A, JSR `__bank_select`. +/// 3. JSR the user-supplied entry label inside the target bank. +/// 4. Pull the saved bank back into A and JSR `__bank_select` to +/// restore the caller's view of $8000-$BFFF. +/// 5. RTS. +/// +/// The save/restore via `ZP_BANK_CURRENT + PHA/PLA` makes the same +/// trampoline work for **fixed-bank → switchable-bank** *and* +/// **switchable-bank → switchable-bank** call directions: the +/// caller's bank ends up restored regardless of where the call +/// originated. Nested cross-bank calls compose because each +/// trampoline's PHA/PLA pair is balanced against its own JSR/RTS, +/// so the saved bank values stack like any other 6502 frame. +/// +/// The trampoline body itself lives in the fixed bank, which is +/// always mapped at `$C000-$FFFF`, so it's reachable from every +/// switchable bank without further mapper trickery. /// /// `tramp_label` is the label that callers will JSR (the IR codegen /// emits `JSR __tramp_` at every cross-bank call site). @@ -1531,17 +1562,21 @@ pub fn gen_bank_select(mapper: Mapper) -> Vec { /// callee's first instruction — conventionally `__ir_fn_`, /// the same label IR codegen would have emitted for an in-bank call. /// `bank_index` is the physical PRG bank number of the target bank. -/// `fixed_bank_index` is the physical bank number of the fixed bank -/// (always `total_banks - 1`). #[must_use] pub fn gen_bank_trampoline( tramp_label: &str, entry_label: &str, bank_index: u8, - fixed_bank_index: u8, ) -> Vec { let mut out = Vec::new(); out.push(Instruction::new(NOP, AM::Label(tramp_label.to_string()))); + // Save the caller's current bank. `__bank_select` writes its + // input into ZP_BANK_CURRENT, so this slot already mirrors the + // last-selected bank (initialized to the fixed bank index by + // `gen_mapper_init` so even fixed-bank callers see a sane + // value the first time around). + out.push(Instruction::new(LDA, AM::ZeroPage(ZP_BANK_CURRENT))); + out.push(Instruction::implied(PHA)); // Switch to target bank. out.push(Instruction::new(LDA, AM::Immediate(bank_index))); out.push(Instruction::new(JSR, AM::Label("__bank_select".into()))); @@ -1549,8 +1584,10 @@ pub fn gen_bank_trampoline( // the switchable bank and is resolved by the linker after the // banked code is assembled. out.push(Instruction::new(JSR, AM::Label(entry_label.to_string()))); - // Restore the fixed bank. - out.push(Instruction::new(LDA, AM::Immediate(fixed_bank_index))); + // Restore the caller's bank (pulled from the stack) so control + // returns with $8000-$BFFF showing whatever the caller had + // mapped before the trampoline ran. + out.push(Instruction::implied(PLA)); out.push(Instruction::new(JSR, AM::Label("__bank_select".into()))); out.push(Instruction::implied(RTS)); out diff --git a/src/runtime/tests.rs b/src/runtime/tests.rs index 4301955..0760e48 100644 --- a/src/runtime/tests.rs +++ b/src/runtime/tests.rs @@ -574,17 +574,31 @@ fn mapper_init_mmc1_horizontal_vs_vertical_control_bits() { } #[test] -fn mapper_init_uxrom_emits_label_and_nothing_else() { +fn mapper_init_uxrom_emits_label_and_seeds_zp_bank_current() { // UxROM powers up with bank 0 at $8000 and the last bank fixed - // at $C000 — exactly what the NEScript runtime expects. All we - // need is a marker label so debuggers can find the (empty) - // init span. + // at $C000, so apart from a marker label there's no mapper- + // specific init to do — *but* the runtime now seeds + // ZP_BANK_CURRENT with the fixed bank index so the + // banked-call trampoline knows which bank to restore on the + // way out. The seed is a 2-instruction LDA #imm / STA $10 + // pair appended after the marker label. let init = gen_mapper_init(Mapper::UxROM, Mirroring::Horizontal, 3); - assert_eq!(init.len(), 1); assert!( matches!(&init[0].mode, AM::Label(n) if n == "__uxrom_init"), - "UxROM init should emit just the marker label", + "UxROM init should still start with the marker label", ); + assert_eq!( + init.len(), + 3, + "UxROM init should be marker + LDA #fixed + STA ZP_BANK_CURRENT" + ); + assert_eq!(init[1].opcode, LDA); + assert!( + matches!(init[1].mode, AM::Immediate(2)), + "fixed bank index for 3 banks is 2" + ); + assert_eq!(init[2].opcode, STA); + assert!(matches!(init[2].mode, AM::ZeroPage(addr) if addr == ZP_BANK_CURRENT)); } #[test] @@ -763,17 +777,18 @@ fn bank_select_assembles_for_every_mapper() { } #[test] -fn trampoline_switches_target_then_restores_fixed() { - // A trampoline must JSR `__bank_select` twice: once with the - // target bank's index, once with the fixed bank's index. The - // two LDA immediates in the stub should match those two bank - // numbers in order. The trampoline name is the label callers - // will JSR (one trampoline per banked function); the entry - // label is whatever lives in the switchable bank. - let t = gen_bank_trampoline("__tramp_helper", "__ir_fn_helper", 0, 3); +fn trampoline_switches_target_then_restores_caller() { + // A trampoline must save the caller's bank from + // ZP_BANK_CURRENT, switch to the target, call the entry, then + // restore the saved value. We check the immediate loaded + // (target bank), the PHA/PLA pair around the body, and the + // JSR sequence. + let t = gen_bank_trampoline("__tramp_helper", "__ir_fn_helper", 0); // First instruction is the trampoline label. assert!(matches!(&t[0].mode, AM::Label(n) if n == "__tramp_helper")); - // Extract the sequence of immediate loads. + // The only LDA immediate is the target bank itself — the + // restore path uses PLA, not a hardcoded LDA, so the caller's + // bank can be anything. let imms: Vec = t .iter() .filter_map(|i| { @@ -785,8 +800,29 @@ fn trampoline_switches_target_then_restores_fixed() { None }) .collect(); - assert_eq!(imms, vec![0, 3], "trampoline should load target then fixed"); - // And two JSRs to __bank_select, plus one JSR to the entry. + assert_eq!( + imms, + vec![0], + "trampoline must load only the target bank as an immediate" + ); + // The trampoline must read ZP_BANK_CURRENT into A then PHA + // (save), and later PLA (restore). + let reads_current = t.iter().any(|i| { + i.opcode == LDA && matches!(i.mode, AM::ZeroPage(addr) if addr == ZP_BANK_CURRENT) + }); + assert!( + reads_current, + "trampoline must LDA ZP_BANK_CURRENT to capture the caller's bank" + ); + let pushes = t.iter().filter(|i| i.opcode == PHA).count(); + let pops = t.iter().filter(|i| i.opcode == PLA).count(); + assert_eq!( + (pushes, pops), + (1, 1), + "trampoline must have exactly one PHA / PLA pair" + ); + // And two JSRs to __bank_select, plus one JSR to the entry — + // dispatch order is still target-first, restore-last. let jsrs: Vec<&str> = t .iter() .filter_map(|i| { @@ -801,7 +837,7 @@ fn trampoline_switches_target_then_restores_fixed() { assert_eq!( jsrs, vec!["__bank_select", "__ir_fn_helper", "__bank_select"], - "trampoline JSRs must dispatch in the correct order" + "trampoline JSRs must dispatch target → entry → restore" ); // Final instruction returns to caller. assert_eq!(t.last().unwrap().opcode, RTS); @@ -814,10 +850,58 @@ fn trampoline_label_uses_caller_supplied_name() { // without knowing bank indices. `gen_bank_trampoline` should // emit that exact label as its leading pseudo-op so the // assembler resolves the JSR. - let t = gen_bank_trampoline("__tramp_big_helper", "__ir_fn_big_helper", 1, 3); + let t = gen_bank_trampoline("__tramp_big_helper", "__ir_fn_big_helper", 1); assert!(matches!(&t[0].mode, AM::Label(n) if n == "__tramp_big_helper")); } +#[test] +fn mapper_init_seeds_zp_bank_current_with_fixed_bank_index() { + // The trampoline reads ZP_BANK_CURRENT to decide which bank + // to switch back to after a cross-bank call. For fixed-bank + // callers that have never explicitly switched banks, the + // value must point at the fixed bank — otherwise the very + // first cross-bank call from the fixed bank would restore + // bank 0 (the RAM-clear default) at $8000, breaking the + // pre-banked-banked semantics that other examples rely on. + // + // For UxROM with 6 banks, the fixed bank is index 5. + let init = gen_mapper_init(Mapper::UxROM, Mirroring::Horizontal, 6); + let mut found_seed = false; + let mut last_imm: Option = None; + for inst in &init { + if inst.opcode == LDA { + if let AM::Immediate(v) = inst.mode { + last_imm = Some(v); + } + } + if inst.opcode == STA { + if let AM::ZeroPage(addr) = inst.mode { + if addr == ZP_BANK_CURRENT && last_imm == Some(5) { + found_seed = true; + break; + } + } + } + } + assert!( + found_seed, + "gen_mapper_init must seed ZP_BANK_CURRENT with the fixed bank index (5 here)" + ); +} + +#[test] +fn nrom_mapper_init_does_not_seed_zp_bank_current() { + // NROM has no banks at all, so seeding ZP_BANK_CURRENT would + // both waste a couple of bytes and cause the existing NROM + // example ROMs (every flat-mapper sample) to byte-shift. + // Verify the init stays empty for NROM. + let init = gen_mapper_init(Mapper::NROM, Mirroring::Horizontal, 1); + assert!( + init.is_empty(), + "NROM mapper init must remain empty: {init:?}" + ); +} + #[test] fn uxrom_bank_table_is_256_bytes_of_sequential_values() { // The bus-conflict table must contain bytes 0..=255 in order diff --git a/tests/emulator/goldens/uxrom_banked_to_banked.audio.hash b/tests/emulator/goldens/uxrom_banked_to_banked.audio.hash new file mode 100644 index 0000000..5f988a9 --- /dev/null +++ b/tests/emulator/goldens/uxrom_banked_to_banked.audio.hash @@ -0,0 +1 @@ +a82b6ff5 132084 diff --git a/tests/emulator/goldens/uxrom_banked_to_banked.png b/tests/emulator/goldens/uxrom_banked_to_banked.png new file mode 100644 index 0000000..af43cac Binary files /dev/null and b/tests/emulator/goldens/uxrom_banked_to_banked.png differ