From 91f82c169a21d3a054940dcb88d7e91714caf541 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 20 Apr 2026 15:15:19 +0000 Subject: [PATCH] runtime: stop `__divide` / `__multiply` from stomping ZP_CURRENT_STATE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: `ZP_DIV_REMAINDER`, `ZP_MUL_RESULT_HI`, and `ZP_CURRENT_STATE` all live at `$03`. The divide routine was zeroing the byte on entry (`LDA #0; STA $03`) and writing the running remainder there on every one of its 8 iterations; the multiply routine accumulated its running product there. Any multi-state program doing `u8 / 10` in `on_frame` had its state ID clobbered on the way out of the routine — the next main-loop dispatch read `$03 == 0` (or whatever the remainder happened to be), matched state 0, and handed control to `Title` instead of the current state. The platformer's HUD hit this once per blink period: Playing survived exactly one frame, then Title took over for 20 frames, then the cycle repeated. Fix: rewrite both runtime routines to keep their running accumulators in register A instead of `$03`. The new contracts: - `__divide`: input `A = dividend`, `$02 = divisor`. Output `A = remainder`, `$04 = quotient`. The algorithm shifts the dividend-turning-into-quotient through `$04` (same as before) and rotates the extracted bits into `A`, comparing and subtracting directly without ever touching `$03`. - `__multiply`: input `A = multiplicand`, `$02 = multiplier`. Output `A = product` (low 8 bits — high byte discarded for `u8 * u8 → u8` as before, but not via a `$03` write). The multiplicand gets shifted left each iteration via `$04` and the running sum stays in `A`. `IrOp::Div` lowering gains one extra `LDA $04` after the JSR to pick up the quotient; `IrOp::Mod` loses the old `LDA $03` since the remainder is already in A. Net callsite cost is one instruction either way. Added two regression tests — `divide_routine_does_not_touch_zp_03` and `multiply_routine_does_not_touch_zp_03` — that walk the emitted instruction stream and fail loudly on any ZeroPage($03) access, so a future refactor can't silently reintroduce the alias. Rebuilt the three ROMs that use `/` or `*` (bitwise_ops, mmc1_banked, platformer) and re-baselined the platformer audio golden — the new instruction count shifts vblank-relative audio timing by a few cycles, as the CLAUDE.md audio-churn note warns. Pixel goldens and docs/platformer.gif stay byte-identical. The platformer HUD is back on native `stomp_count / 10` + `% 10`; the subtraction-loop workaround is gone. docs/future-work.md gains a new section describing the planned sprite-0 hit upgrade for the platformer HUD (carry-over task from the branch). --- docs/future-work.md | 67 +++++++------ examples/bitwise_ops.nes | Bin 24592 -> 24592 bytes examples/mmc1_banked.nes | Bin 57360 -> 57360 bytes examples/platformer.ne | 33 +------ examples/platformer.nes | Bin 24592 -> 24592 bytes src/codegen/ir_codegen.rs | 25 +++-- src/runtime/mod.rs | 99 ++++++++++++------- src/runtime/tests.rs | 42 ++++++++ tests/emulator/goldens/platformer.audio.hash | 2 +- 9 files changed, 160 insertions(+), 108 deletions(-) diff --git a/docs/future-work.md b/docs/future-work.md index f2473e1..765d69f 100644 --- a/docs/future-work.md +++ b/docs/future-work.md @@ -260,41 +260,46 @@ missing: call — would shave a few bytes more on programs with many deep handlers. -### `u8 / 10` and `u8 % 10` miscompile near state transitions +### `examples/platformer.ne` HUD — move from sprites to sprite-0 hit -The `examples/platformer.ne` HUD originally rendered its two-digit -stomp tally with `TILE_DIGIT_0 + (stomp_count / 10)` and -`TILE_DIGIT_0 + (stomp_count % 10)`. Both expressions passed the -parser, survived analysis, and emitted a W0101 "expensive -multiply/divide" warning as expected — but the built ROM cycled -Title → Playing → Title once per blink period, with the Playing -state surviving for exactly one frame before something stomped the -state-machine bookkeeping at `$1B` (Title's `blink` / Playing's -`player_y` overlay slot) and kicked control back to Title. The -divide has to be the culprit: swapping both calls for a manual -`while r >= 10 { r -= 10; … }` in two helper functions makes the -golden hold across every frame. +The platformer currently renders its status bar as five OAM +sprites re-emitted every frame inside `draw_hud()`. That works but +burns 5/64 OAM slots on static UI and forces the redraw path to +walk the HUD every tick even when `score` / `lives` haven't +changed. The clean upgrade is to paint the HUD into the top row of +the nametable and use a sprite-0 hit split (same pattern as +`examples/sprite_0_split_demo.ne`) to reset the X-scroll at the +HUD/playfield boundary so the rest of the screen can keep +scrolling freely. Target shape: -The HUD's `draw_hud` workaround sits in `examples/platformer.ne` -with a comment pointing here. Two guesses at the root cause: +1. Pre-paint row 0 of the nametable with `Bar` + glyph tiles via + an `on enter` one-shot that drops `nt_set` / `nt_fill_h` + writes into the VRAM ring — same shape as + `examples/hud_demo.ne`. +2. Place sprite 0 as a 1-pixel-wide invisible dot on the last + scanline of the HUD row; the PPU sets the sprite-0 hit flag + when rendering crosses it. +3. Poll the flag in `on frame` and latch `scroll(camera_x, 0)` + only after the hit, leaving the HUD painted with scroll=0 for + the top rows. +4. Replace the four in-frame `draw Tileset at: (…HUD…)` calls + with a shadow-compare `if score != last_score { nt_set(...) }` + pair (score digits) + `if lives != last_lives { nt_set(...) }` + (heart row). Most frames write nothing. +5. The palette row at metatile y=0 needs a dedicated sub-palette + so the HUD doesn't inherit the sky/brick/ground attribute + zones — add it to `palette_map` in row 0. +6. Shadow variables `last_score` / `last_lives` initialize to + `255` so the first frame paints unconditionally. -1. **Scratch overlap.** The divide routine's scratch may land - beyond the `$00-$0F` runtime reservation and stomp on the - state-local overlay base at `$1B`. A printout of the runtime's - actual zero-page footprint alongside the analyzer's allocator - snapshot would catch this. -2. **Expression lowering.** The `BinaryOp::Div` / `Rem` path may - leave a dangling temp in a slot that a surrounding `draw` - statement later clobbers. The existing - `uninitialized_struct_field_store_emits_sta_to_allocated_address` - regression test exercises loads + stores but not divides; a - round-trip test along the lines of "compile `x / 10` into a - `var tens: u8`, wait a frame, assert `tens` still reads as the - pre-wait value" would pin this down. +Wins: 5 OAM slots back for enemies/particles, no per-frame HUD +draw cost, and the flagship demo actually exercises sprite-0 hit +in a real game (the standalone `sprite_0_split_demo.ne` stays as +the minimal pedagogical example). Still NROM — no mapper change. -Either way the negative test is the priority — without one, the -next person to reach for `/` or `%` in a state with overlaid -locals hits the same PR #31-shaped silent drop. +Blocker to track: the HUD reads `stomp_count` as a u8 decimal; +when this lands, plumb it through whatever digit-extraction path +replaces the current `tens_digit` / `ones_digit` helpers. ### Cross-block temp live-range analysis diff --git a/examples/bitwise_ops.nes b/examples/bitwise_ops.nes index cb667cf6a8d7020be1b524788bf57a0baa1f329d..39306b8da5555f97975dae857cc309505ba7f027 100644 GIT binary patch delta 49 zcmbPmfN=s4)i9phSR2H|w}``yX#%rEDhnISsS95=KV?$2ogBc-!TRnH!vT%W{0@u? E0LixzRsaA1 delta 53 zcmbPmfN=s4)i7S$SR2G7*2=ty!;NVIyTnrFRF+m|HkMNtJ}hP4e3wbpmi5~qh64th I`5hP)01c=T-~a#s diff --git a/examples/mmc1_banked.nes b/examples/mmc1_banked.nes index a3bbd7d001f7484da6164b67639f8cb676c9cc71..ed5a8ce016edb0f779a916fbb0f55e3617e262d3 100644 GIT binary patch delta 50 zcmV-20L}l9zypxL1F)We0Z+4^fUyA%q6kI;kOLTH1O^1k(Dburf}}l>AP5C-!T`Xj Ivk$-lAp8*%YybcN delta 55 zcmV-70LcH4zypxL1F)We0avr1fUyA u8 { - var t: u8 = 0 - var r: u8 = n - while r >= 10 { - r -= 10 - t += 1 - } - return t -} - -fun ones_digit(n: u8) -> u8 { - var r: u8 = n - while r >= 10 { - r -= 10 - } - return r -} - fun draw_hud() { // Score side (left): coin + tens + ones. draw Tileset at: (16, 16) frame: TILE_COIN - draw Tileset at: (28, 16) frame: TILE_DIGIT_0 + tens_digit(stomp_count) - draw Tileset at: (36, 16) frame: TILE_DIGIT_0 + ones_digit(stomp_count) + draw Tileset at: (28, 16) frame: TILE_DIGIT_0 + (stomp_count / 10) + draw Tileset at: (36, 16) frame: TILE_DIGIT_0 + (stomp_count % 10) // Lives side (right): heart + digit. draw Tileset at: (208, 16) frame: TILE_HEART @@ -843,8 +818,8 @@ state GameOver { // final score and the remaining heart total are visible at // the exact same position as in Playing — no awkward jump. draw Tileset at: (16, 16) frame: TILE_COIN - draw Tileset at: (28, 16) frame: TILE_DIGIT_0 + tens_digit(stomp_count) - draw Tileset at: (36, 16) frame: TILE_DIGIT_0 + ones_digit(stomp_count) + draw Tileset at: (28, 16) frame: TILE_DIGIT_0 + (stomp_count / 10) + draw Tileset at: (36, 16) frame: TILE_DIGIT_0 + (stomp_count % 10) draw Tileset at: (208, 16) frame: TILE_HEART draw Tileset at: (224, 16) frame: TILE_DIGIT_0 + lives diff --git a/examples/platformer.nes b/examples/platformer.nes index 271a1794040f0ddd2153a0d0fc02e4a1046d062f..f73662e4f1c94f1ceaaa8812a1be0198ad14d1a6 100644 GIT binary patch delta 1075 zcmZ`%O=uHA7~PrO{5FZvLrL5;ZkjYDRcjFnf<=NNl(H=e=shTfp1gPv!P0b-p$93X z?x6;;1a+w6sZu<6OOs8L6$KA)tMqz$3PqH*P z&fEmgNX4f!BCBQK6#F0|)~+ z_n9Z6++TrEk)h2#IR%G-=`ay#SQiSse7@2O1HT0PRE3pH76}16oPl~o=!KyoQcIX5 z(p4(x83+L&>i4Z&7F=teg**9&)hWJru&XqN+QpfXpd4YP!v-AcUA`Y3RZ~K=R|M5D zDC2=iGB;t+t*9UQc<@aXs^X+Z*bvFgmxa(U6V|UIq}= zuLD@nr0tDXKaE+tWpl?+t6$gC|A<71Aib^7n6ADT8o^k|++wSc4HMkR;og>PDJC#4&d3A2JZe`%*hjmEEOIOT0l2_}0U6|BiRl8YR zT`Y(q0^=Sz?ZQwE7la+$5?(dGWvJT_OWbo$(*wTel90nK3k^UmPZPJiqtubW#;whs z=1pMvnoeK|NjcTf_l?VPwDAo7gD`x@CMIlx%lyj}^_p|&*n9&{k0zPV}DZm4UWaOkpKVy delta 1056 zcmZ`%O=uHA6yDj{q{*7rsEA3oNw(o)a?M-VDKddbfw?NZcUdh=KUJ&3CZFP7G~o3sr|nPumjdEb2V-kUF*M_Kru zp{sMxRWv!b(1SvAdg$J4i3#R57pTCnA*^Hap=2PK#&ko3>3W5H%R@B6BpPK=Lter} zDX{3^mQ1772yqrzBmuD)i`lV(0*hI(80iF*&{|OB6qVGSJWaEtZz4@+-n!XzP7v9Q zHjaFe8SbUiG{yWt3KnRRrF;`fn%*O&b4cVuPPFLprn6cF^@`_<+7EPc>;(02gEsSB zDkoUL5KIwLyIW+6fdSkAFt~-OLT(o!eTAb+fTUs5HxZ-?3mQ@ulgGts5crG0Pir^D zg(+HMl97N20L6G|OWJU$wim2lbHBu$GyuB6s-O2|#5i|laGW=!#8E9NOA2@$d~u(p z9wg3%gq0i&AIG#+dxG64qP0m~+B|l(e6>z0kVp`2|E(?Kkl(FyM6ueY!>ce9_L_2i zt^LAb`)E{@i`uX>Ftw($HUpWM0O&Fv0QjVND6?Tz%ggIynbJC-vcbU32CA72rsQJY? P=j3N0@A&e@8(04VX~U+F diff --git a/src/codegen/ir_codegen.rs b/src/codegen/ir_codegen.rs index e969692..0ee971b 100644 --- a/src/codegen/ir_codegen.rs +++ b/src/codegen/ir_codegen.rs @@ -1514,11 +1514,18 @@ impl<'a> IrCodeGen<'a> { IrOp::ShiftRightVar(d, a, amt) => self.gen_shift_var(*d, *a, *amt, LSR), IrOp::Div(d, a, b) => { // Software divide: dividend in A, divisor in $02. - // `__divide` returns quotient in A and leaves - // remainder in ZP $03. Flag the `__div_used` marker - // so the linker links the `__divide` subroutine in; + // `__divide` returns the quotient in ZP $04 and the + // remainder in A. Flag the `__div_used` marker so + // the linker links the `__divide` subroutine in; // programs that don't divide skip ~70 bytes. The - // label is emitted at the end of generate(). + // label itself is emitted at the end of generate(). + // + // We deliberately read the quotient from $04 (not + // A) so the routine can keep the running remainder + // in the accumulator and leave `$03` alone — `$03` + // is `ZP_CURRENT_STATE`, and touching it would + // stomp the state-machine dispatch. See + // `runtime::gen_divide` for the contract. self.div_used = true; self.load_temp(*a); self.emit(PHA, AM::Implied); @@ -1527,13 +1534,14 @@ impl<'a> IrCodeGen<'a> { self.emit(STA, AM::ZeroPage(0x02)); self.emit(PLA, AM::Implied); self.emit(JSR, AM::Label("__divide".into())); + self.emit(LDA, AM::ZeroPage(0x04)); self.store_temp(*d); } IrOp::Mod(d, a, b) => { - // Modulo reuses __divide and reads the remainder out - // of ZP $03 afterwards. Same `__div_used` marker as - // `IrOp::Div` — modulo doesn't have a separate - // runtime routine. + // Modulo reuses __divide and picks up the remainder + // from the accumulator afterwards. Same `__div_used` + // marker as `IrOp::Div` — modulo doesn't have a + // separate runtime routine. self.div_used = true; self.load_temp(*a); self.emit(PHA, AM::Implied); @@ -1542,7 +1550,6 @@ impl<'a> IrCodeGen<'a> { self.emit(STA, AM::ZeroPage(0x02)); self.emit(PLA, AM::Implied); self.emit(JSR, AM::Label("__divide".into())); - self.emit(LDA, AM::ZeroPage(0x03)); self.store_temp(*d); } IrOp::Negate(d, src) => { diff --git a/src/runtime/mod.rs b/src/runtime/mod.rs index cf7cbe6..61ae784 100644 --- a/src/runtime/mod.rs +++ b/src/runtime/mod.rs @@ -979,31 +979,44 @@ pub fn gen_initial_background_load(tiles_label: &str, attrs_label: &str) -> Vec< out } -/// Zero-page locations used by multiply/divide routines. +/// Zero-page locations used by the multiply / divide routines. +/// `$02` is the "second operand" input slot — the multiplier for +/// `__multiply` and the divisor for `__divide`. `$04` is the +/// routines' private scratch: it holds the working multiplicand +/// (shifted left each iteration) for multiply and the running +/// dividend-turning-into-quotient for divide. Both routines keep +/// their running output in the accumulator, so `$03` +/// (`ZP_CURRENT_STATE` in the codegen) stays untouched — aliasing +/// `$03` across the divide routine used to zero it and hand +/// control to state 0 on the next frame. const ZP_MUL_OPERAND: u8 = 0x02; -const ZP_MUL_RESULT_HI: u8 = 0x03; const ZP_DIV_DIVISOR: u8 = 0x02; -const ZP_DIV_REMAINDER: u8 = 0x03; -/// Generate 8x8 -> 16 software multiply routine. +/// Generate 8x8 -> 8 software multiply routine. /// /// Input: A = multiplicand, zero-page $02 = multiplier -/// Output: A = result low byte, $03 = result high byte +/// Output: A = product (low byte; high byte is discarded for +/// unsigned u8 * u8 → u8 semantics) /// /// Algorithm: shift-and-add. For each bit of the multiplier, if set, -/// add the (shifted) multiplicand to the result. +/// add the (shifted) multiplicand to the running product. The +/// product is kept in the accumulator — not in ZP `$03` — so the +/// routine doesn't alias with `ZP_CURRENT_STATE`. `$02` is read- +/// only at the callsite (the divisor/multiplier input slot) and is +/// shifted in place here, which is fine because the caller re- +/// populates it on every `__multiply` call. pub fn gen_multiply() -> Vec { let mut out = Vec::new(); // Label for the subroutine entry out.push(Instruction::new(NOP, AM::Label("__multiply".into()))); - // Store multiplicand in $04 (working copy) + // Store multiplicand in $04 (working copy; we shift it left + // each iteration to line up with the next multiplier bit). out.push(Instruction::new(STA, AM::ZeroPage(0x04))); - // Clear result: A (low) and $03 (high) + // Running product starts at 0 in A. out.push(Instruction::new(LDA, AM::Immediate(0x00))); - out.push(Instruction::new(STA, AM::ZeroPage(ZP_MUL_RESULT_HI))); // Loop counter: 8 bits out.push(Instruction::new(LDX, AM::Immediate(0x08))); @@ -1011,24 +1024,24 @@ pub fn gen_multiply() -> Vec { // __mul_loop: out.push(Instruction::new(NOP, AM::Label("__mul_loop".into()))); - // Shift multiplier right, check carry (current bit) + // Shift multiplier right, check carry (current bit). out.push(Instruction::new(LSR, AM::ZeroPage(ZP_MUL_OPERAND))); out.push(Instruction::new( BCC, AM::LabelRelative("__mul_no_add".into()), )); - // Carry set: add multiplicand to result - // Add low byte + // Carry set: add the current shifted multiplicand to the + // running product in A. CLC first because LSR sets carry to the + // shifted-out bit. out.push(Instruction::implied(CLC)); - out.push(Instruction::new(LDA, AM::ZeroPage(ZP_MUL_RESULT_HI))); out.push(Instruction::new(ADC, AM::ZeroPage(0x04))); - out.push(Instruction::new(STA, AM::ZeroPage(ZP_MUL_RESULT_HI))); // __mul_no_add: out.push(Instruction::new(NOP, AM::Label("__mul_no_add".into()))); - // Shift multiplicand left (double it) for next bit position + // Shift multiplicand left (double it) for the next bit + // position. out.push(Instruction::new(ASL, AM::ZeroPage(0x04))); // Decrement counter @@ -1038,11 +1051,7 @@ pub fn gen_multiply() -> Vec { AM::LabelRelative("__mul_loop".into()), )); - // Load low byte of result into A - // For 8-bit result, just use the high byte accumulation - // (since we shifted the multiplicand left, result is in $03) - out.push(Instruction::new(LDA, AM::ZeroPage(ZP_MUL_RESULT_HI))); - + // A already holds the product. out.push(Instruction::implied(RTS)); out @@ -1610,19 +1619,33 @@ pub fn gen_data_block(label: &str, bytes: Vec) -> Vec { /// Generate 8 / 8 -> 8 software divide routine (restoring division). /// /// Input: A = dividend, zero-page $02 = divisor -/// Output: A = quotient, $03 = remainder +/// Output: A = remainder, zero-page $04 = quotient +/// +/// Keeps the running remainder in the accumulator (instead of ZP +/// $03) so the routine doesn't alias with `ZP_CURRENT_STATE` at +/// $03. Any program with a state machine reads $03 to dispatch +/// `on_frame`; the old shape zeroed it on every divide call and +/// kicked control back to state 0 (usually `Title`) on the very +/// next frame. The only caller-visible change is that `IrOp::Div` +/// now picks up the quotient from `$04` rather than `A` — see +/// `ir_codegen.rs` for the Div/Mod callsite shape. `$04` stays +/// inside the `$00-$0F` runtime reservation so it never collides +/// with user variables (analyzer starts user ZP at `$10` minimum). pub fn gen_divide() -> Vec { let mut out = Vec::new(); // Label for the subroutine entry out.push(Instruction::new(NOP, AM::Label("__divide".into()))); - // Store dividend in $04 + // Stash dividend in $04. Over the course of the loop we shift + // it left 8 times; the bits that fall out become the running + // remainder (in A), and the bits shifted in (via `INC $04` + // below) become the quotient, so $04 ends up holding exactly + // the quotient we want to return. out.push(Instruction::new(STA, AM::ZeroPage(0x04))); - // Clear remainder + // Running remainder starts at 0 in A. out.push(Instruction::new(LDA, AM::Immediate(0x00))); - out.push(Instruction::new(STA, AM::ZeroPage(ZP_DIV_REMAINDER))); // Loop counter: 8 bits out.push(Instruction::new(LDX, AM::Immediate(0x08))); @@ -1630,25 +1653,27 @@ pub fn gen_divide() -> Vec { // __div_loop: out.push(Instruction::new(NOP, AM::Label("__div_loop".into()))); - // Shift dividend left into remainder + // Shift the next bit of the dividend out of $04 into the carry, + // then rotate it into the remainder accumulator. out.push(Instruction::new(ASL, AM::ZeroPage(0x04))); - out.push(Instruction::new(ROL, AM::ZeroPage(ZP_DIV_REMAINDER))); + out.push(Instruction::new(ROL, AM::Accumulator)); - // Try to subtract divisor from remainder - out.push(Instruction::new(LDA, AM::ZeroPage(ZP_DIV_REMAINDER))); - out.push(Instruction::implied(SEC)); - out.push(Instruction::new(SBC, AM::ZeroPage(ZP_DIV_DIVISOR))); - - // If remainder >= divisor (no borrow), keep subtraction + // Remainder >= divisor? CMP sets carry when A >= memory, clear + // when A < memory. On `>=` we do the subtraction for real with + // SBC (which still sees the same carry-set condition) and set + // the quotient bit for this iteration. On `<` we leave A alone. + out.push(Instruction::new(CMP, AM::ZeroPage(ZP_DIV_DIVISOR))); out.push(Instruction::new( BCC, AM::LabelRelative("__div_no_sub".into()), )); - // Store updated remainder - out.push(Instruction::new(STA, AM::ZeroPage(ZP_DIV_REMAINDER))); + // Accept the subtraction into the remainder. SBC with carry set + // performs a true subtract (no extra borrow). + out.push(Instruction::new(SBC, AM::ZeroPage(ZP_DIV_DIVISOR))); - // Set bit 0 of quotient (in $04, which we shifted left) + // Set bit 0 of the quotient (stored in $04, whose low bit is + // free because we just shifted left). out.push(Instruction::new(INC, AM::ZeroPage(0x04))); // __div_no_sub: @@ -1661,9 +1686,7 @@ pub fn gen_divide() -> Vec { AM::LabelRelative("__div_loop".into()), )); - // Load quotient into A - out.push(Instruction::new(LDA, AM::ZeroPage(0x04))); - + // A already holds the final remainder. $04 holds the quotient. out.push(Instruction::implied(RTS)); out diff --git a/src/runtime/tests.rs b/src/runtime/tests.rs index 70dcc63..f569c57 100644 --- a/src/runtime/tests.rs +++ b/src/runtime/tests.rs @@ -663,6 +663,48 @@ fn divide_routine_assembles() { ); } +/// `$03` is `ZP_CURRENT_STATE` in the codegen — the byte the main +/// dispatch loop reads every frame to pick the right `on_frame` +/// handler. If the divide routine writes to `$03` at any point it +/// zeroes the current-state value and kicks the state machine back +/// to state 0 on the very next frame (see the PR that introduced +/// the platformer HUD for the full repro). This test pins the +/// routine to a `$03`-free implementation so any future refactor +/// that reintroduces a ZP-3 write fails loudly instead of shipping +/// a silent miscompile. +#[test] +fn divide_routine_does_not_touch_zp_03() { + for inst in gen_divide() { + let touches_03 = matches!(&inst.mode, AM::ZeroPage(0x03)); + assert!( + !touches_03, + "gen_divide emitted an instruction touching $03 \ + (collides with ZP_CURRENT_STATE): {:?} {:?}", + inst.opcode, inst.mode, + ); + } +} + +/// Same argument as `divide_routine_does_not_touch_zp_03`, applied +/// to the multiplicand. The old `gen_multiply` accumulated the +/// result in ZP `$03` even for `u8 * u8 → u8` (the high byte was +/// simply discarded at the callsite), which silently corrupted +/// `ZP_CURRENT_STATE` across every `*` operator in a multi-state +/// program. Holding the running product in the accumulator instead +/// keeps `$03` pristine. +#[test] +fn multiply_routine_does_not_touch_zp_03() { + for inst in gen_multiply() { + let touches_03 = matches!(&inst.mode, AM::ZeroPage(0x03)); + assert!( + !touches_03, + "gen_multiply emitted an instruction touching $03 \ + (collides with ZP_CURRENT_STATE): {:?} {:?}", + inst.opcode, inst.mode, + ); + } +} + // ─── Bank switching ──────────────────────────────────────────────── #[test] diff --git a/tests/emulator/goldens/platformer.audio.hash b/tests/emulator/goldens/platformer.audio.hash index 480e5e7..0f9524b 100644 --- a/tests/emulator/goldens/platformer.audio.hash +++ b/tests/emulator/goldens/platformer.audio.hash @@ -1 +1 @@ -c5e41b59 132084 +30938b26 132084