mirror of
https://github.com/imjasonh/nescript
synced 2026-07-08 17:06:04 +00:00
runtime: stop __divide / __multiply from stomping ZP_CURRENT_STATE
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).
This commit is contained in:
parent
b83b944570
commit
91f82c169a
9 changed files with 160 additions and 108 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue