1
0
Fork 0
mirror of https://github.com/imjasonh/nescript synced 2026-07-18 06:36:57 +00:00
Commit graph

2 commits

Author SHA1 Message Date
Claude
76d0fd0d28
codegen: reuse analyzer's local allocations so inline asm {param} works
Fixes compiler-bugs.md #1 — the inline-asm `{name}` resolver
looks parameters up in the analyzer's `VarAllocation` table
(because that's the only address map it has), but `IrCodeGen::new`
was minting a parallel `$0300+` range for every function-local and
ignoring what the analyzer had picked. The spill prologue wrote the
param to the codegen's private address, the inline asm read from
the analyzer's zero-page address, and nothing ever bridged the two
— `LDA {param}` would silently load whatever the RAM clear left at
the stale slot (always `0`).

Fix: drop the `local_ram_next` loop and just look each local up in
`allocations` by the analyzer's qualified name
(`__local__{scope}__{local}`). The scope string that `gen_function`
already computed for `substitute_asm_vars` is now shared with the
new address-seeding loop via a `scope_prefix_for_fn(&str)` helper,
so the two call sites can't drift. The analyzer's layout already
satisfies the "no overlapping live locals" invariant the codegen
was relying on — it scopes every local under
`__local__<scope>__<name>` so two functions with a parameter named
`x` land in different slots.

Updated `gen_function_prologue_spills_params_to_local_ram`: the
regression test for the War-era param clobbering bug was asserting
the spill's destination specifically had to be an absolute address
at `$0300+`. That's no longer the mechanism — the spill lands in
whatever slot the analyzer assigned, which is zero page when
there's room. The test now asserts the destination is *any*
address outside `$04-$07`, which is the actual invariant.

Reverted the `LDX $04` / `LDY $05` workaround in
`examples/sha256/sha_core.ne` — every primitive there now uses
`{dst}` / `{src}` / `{w_ofs}` / `{h_ofs}` / `{k_ofs}` substitution
as originally intended. The "Parameter convention" comment that
documented the workaround is gone.

Regenerated `tests/emulator/goldens/inline_asm_demo.png`: that
example's `times_four(input)` was previously returning `input`
verbatim because the inline asm's `LDA {result}` / `ASL A` /
`ASL A` / `STA {result}` operated on a zero-page byte that was
disconnected from the NEScript-level `result` variable. With the
fix, `times_four` correctly returns `input * 4`, so the
smiley-tracker's frame-180 position shifts by the expected
`(frame_count * 4) mod 256` delta. The other 33 ROMs remain
byte-identical.

Verified:
  - `cargo clippy --all-targets -- -D warnings` clean on both
    rustc 1.94.1 and 1.95.0.
  - `cargo test --all-targets`: 616 + 3 + 75 tests pass.
  - `cargo fmt --check` clean.
  - Full emulator harness: 34/34 ROMs match goldens.
  - SHA-256 of "NES" still computes to
    `AE9145DB5CABC41FE34B54E34AF8881F462362EA20FD8F861B26532FFBB84E0D`.
  - `--memory-map` output now reflects what the generated code
    actually reads and writes (previously the codegen's $0300+
    override was invisible to the dump).

https://claude.ai/code/session_01FRmSBruVWCufm3LsUVMs8v
2026-04-16 16:03:10 +00:00
Claude
2d66b68c7f
tests/emulator: byte-exact golden-image diffs
The smoke test used to check a per-example `nonBlack` floor — "at
least one sprite rendered," plus a per-example minimum for the
multi-sprite examples. That catches gross regressions (a compiler
bug that makes everything go black) but silently lets through
anything that changes a handful of pixels without dropping below
the sprite floor. The whole point of this harness is to catch
compiler miscompiles before they land; a softer check means bugs
can still sneak in.

This swap makes every run diff the raw canvas framebuffer against
a committed PNG golden. One mismatched byte at pixel (120, 119)
is enough to fail CI — there's nowhere for a regression to hide.

Workflow:

    # normal — fails on any pixel change
    node tests/emulator/run_examples.mjs

    # when the diff is intentional, rewrite the goldens
    UPDATE_GOLDENS=1 node tests/emulator/run_examples.mjs
    # or: node tests/emulator/run_examples.mjs --update-goldens

    git diff tests/emulator/goldens/   # review the change
    git add tests/emulator/goldens/
    git commit                          # explain WHY in the message

When a run fails without `UPDATE_GOLDENS`, the runner writes:

    tests/emulator/actual/<name>.png       the run's raw output
    tests/emulator/actual/<name>.diff.png  red-highlighted pixel diff

so reviewers can eyeball what changed without rerunning locally.
`actual/` is gitignored and re-created on every run. The CI job
now uploads `actual/`, `goldens/`, and `report.json` together as
a single `emulator-diff` artifact on failure — side by side means
the "what changed" story is obvious without cloning.

Implementation:

- `tests/emulator/screenshots/` is renamed to `tests/emulator/goldens/`.
  All 18 existing PNGs are preserved as the initial goldens (git
  detected them as pure renames).

- `harness.html` gets a new `window.nesHarness.rawPixelsBase64()`
  that returns the 245760-byte (256 × 240 × 4 bytes) RGBA canvas
  buffer as base64. The runner compares raw pixels, not PNG
  bytes, so encoder quirks (zlib level, filter heuristics) can't
  cause false positives across Chrome versions or platforms.

- The runner uses `pngjs` (pure-JS, no native deps) to decode
  goldens and to write diff PNGs. `PNG.sync.write` is
  byte-deterministic for identical pixels, so `git diff` on a
  committed golden only ever shows up when the actual rendered
  pixels changed — not because two machines produced slightly
  different compression.

- The committed goldens were re-encoded with pngjs in this commit
  so the baseline is consistent from day one. File sizes are a
  touch larger than Chrome's output (~1KB vs ~800B on average),
  but that's negligible and it eliminates one entire class of
  flaky-looking diffs in the future.

Determinism verification: I ran each of the 18 ROMs twice
through fresh `NES` instances in fresh puppeteer pages, hashed
the 245760-byte framebuffers at frame 180 with SHA-256, and
confirmed `run1 == run2` for every single one. Exact-pixel diffs
are safe for this ROM set.

Negative path verification: I corrupted one golden (flipped one
pixel to pure red via pngjs) and reran the runner. It printed

    DIFF  hello_sprite  1/61440 pixels differ; first at (120,120)
                        expected [255,0,0] got [0,0,0]
            actual: tests/emulator/actual/hello_sprite.png
              diff: tests/emulator/actual/hello_sprite.diff.png

and exited 1 as expected. The diff PNG shows a dim-grayscale
silhouette of the expected frame with a bright-red dot on the
one mismatched pixel — enough visual context to locate the
regression at a glance.

All 18 examples match their goldens in strict mode. `cargo fmt
--check`, `cargo clippy --release --all-targets -- -D warnings`,
and `cargo test --release` (313 unit + 37 integration) are all
still green.

https://claude.ai/code/session_014Z5y3Q9krLcAxYpZQJhZ5V
2026-04-12 21:30:18 +00:00