Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,32 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

- **AccuracySNES: `C7.05` — Range Over trips at the 33rd in-range sprite's evaluation cycle,
`H = OAM.INDEX * 2`.** A fixed bracket would be vacuous: an H-IRQ is serviced ~22-27 dots after its
`HTIME`, so one tight enough to pin dot 65 is inside the latency's own uncertainty and one loose
enough to be safe passes for any set dot. Instead both phases sample the **same** dot and change
only which OAM entry is 33rd in range — index 32 (set dot 65, reads set) against index 72 (set dot
145, reads clear) — so the flag has to *move with the index*. Phase B also re-reads in the same
frame's vblank, since "clear" is otherwise what a core that never sets the flag reports.

Three injections, each failing its own code: a fixed set dot fails code 2, never setting fails
code 1, and setting correctly for a low index but never a high one fails code 3 — the guard phase A
cannot cover.

The sample dot is **measured, not inferred**: an H-IRQ handler runs ~93 dots after its `HTIME`, not
the ~22-27 a raw trigger latency suggests, because the interrupt is only taken at an instruction
boundary and the trampoline, shim and prologue all precede the read.

This needed **new runtime machinery**, which no `v1.28.0` row was supposed to. `irq_trampoline` is
a bank-local `jmp (V_IRQ_VEC)`, so only bank-`$00` groups could install an IRQ handler; Groups
C-G, relocated out of bank `$00`, could not, and none ever had. New `irq_far_shim` plus the 24-bit
`V_IRQ_VEC_FAR` at `$0058` fixes it for every relocated group — `jml` pushes nothing, so a far
handler still ends in a plain `rti`. Default behaviour is unchanged: the shim is opt-in and the far
vector defaults to `irq_stub`.

snes9x fails the row and is recorded as an expected divergence (`SNES9X_KNOWN_FAILURES` 12 → 13):
it reads set in both phases, so its Range Over is scanline-granular rather than per-sprite.

- **AccuracySNES: recorded what each remaining `v1.28.0` row actually needs.** Several rows filed as
"needs no new machinery" need rather more; `A5.18` is parked, `A6.15` needs its own design,
`C7.05`/`C7.06` are reachable via an H-IRQ bracket, and `B2.07`/`B2.09` are `v1.29.0`. The
Expand Down
4 changes: 2 additions & 2 deletions docs/accuracysnes-coverage.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ Every sub-group of Part V is enumerated, so this is a **complete** statement of
| `C4` | 5 | 0 | 5 | — |
| `C5` | 15 | 0 | 12 | C5.06, C5.07, C5.15 |
| `C6` | 7 | 0 | 6 | C6.07 |
| `C7` | 16 | 6 | 6 | C7.05, C7.06, C7.07, C7.12 |
| `C7` | 16 | 7 | 6 | C7.06, C7.07, C7.12 |
| `C8` | 12 | 0 | 11 | C8.09 |
| `C9` | 8 | 2 | 0 | C9.01, C9.02, C9.03, C9.06, C9.07, C9.08 |
| `C10` | 5 | 0 | 3 | C10.03, C10.04 |
Expand All @@ -52,7 +52,7 @@ Every sub-group of Part V is enumerated, so this is a **complete** statement of
| `F1` | 22 | 13 | 0 | F1.13, F1.15, F1.16, F1.17, F1.18, F1.19, F1.20, F1.21, F1.22 |
| `G1` | 18 | 15 | 0 | G1.06, G1.13, G1.18 |

**292 of 443** enumerated assertion rows covered by an on-cart test, plus **53** covered only by a rendered scene (`docs/adr/0013`) — **345 of 443** in total.
**293 of 443** enumerated assertion rows covered by an on-cart test, plus **53** covered only by a rendered scene (`docs/adr/0013`) — **346 of 443** in total.

The two columns are kept apart on purpose. An on-cart result means the same thing on any emulator and on real hardware; a rendered scene needs a host holding the golden. Adding them into one figure would quietly change what the number claims.

Expand Down
49 changes: 47 additions & 2 deletions docs/accuracysnes-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,8 @@ before the H counter wraps and silently returns a plausible small number.
| `A5.19` | **landed** | see below |
| `A5.18` | **parked** — needs a cheaper instrument | The design is sound and the numbers came out exactly as predicted, but a `BRK` round trip is 39 dots and only 3 fit inside the budget, leaving 6 dots of signal against `TOL` 2 in a difference-of-differences. See below. |
| `A6.15` | needs its own design | "all 256 opcodes defined" is a *definedness* assertion, not a timing one, and `sweep.rs` deliberately covers only the unambiguous subset — its own module docs list why control flow, `BRK`/`COP`, `STP`, `WAI` and memory-addressing modes are excluded. Citing the sweep would be coverage by restatement. |
| `C7.05`, `C7.06` | **reachable — sample with an H-IRQ bracket** | Not an accuracy gap: re-measured, `range_over` first reads set at scanline **100**, MesenCE's value. What remains is sampling *resolution*. See below. |
| `C7.05` | **landed** | Not a fixed bracket — the set dot has to *move with the index*. See below. |
| `C7.06` | reachable, same technique | `C7.05`'s far-IRQ shim and sprite setup are reusable; `time_over` needs >34 sprite-tiles, which 40 8x8 sprites do **not** produce (range evaluation caps at 32, so 32 tiles). Use larger sprites. |
| `B2.07` | gated on `B2.02` | `B2.04` covers the 262-line count, not the clock total, so the frequency is not implied by it. Getting to 60.0988 Hz needs clocks-per-frame, and the frame alternates 357,368/357,364 because of the short scanline — which *is* `B2.02`, a `T-06-A` dot-model residual scheduled for `v1.29.0`. |
| `B2.09` | `v1.29.0` by its own dossier note | the picture window is "not CPU-observable directly; reachable through the framebuffer oracle once the dot-resolution compositor lands". |

Expand All @@ -495,7 +496,51 @@ Injecting at `op_brk`'s PBR push moved **no** timing at all — it broke `A6.13`
— so RustySNES's `BRK` cycle cost does not come from that bus access. Trace where it does come from
before designing this row's injection test.

#### `C7.05`/`C7.06` — why the bracket, and the two traps
#### `C7.05` — a fixed bracket is vacuous; move the index instead

An H-IRQ handler does not run where its `HTIME` says. **Measured** — by temporarily latching
`$213C` inside the handler — the read lands ~**93 dots** later: the interrupt is only taken at an
instruction boundary in `wait_vblank_far`'s poll loop, and the trampoline, the far shim and the
handler prologue all precede it. So a bracket tight enough to pin dot 65 is inside the latency's own
uncertainty, and one loose enough to be safe passes for any set dot across most of the line — the
`F1.09` shape.

What pins `H = OAM.INDEX * 2` is that the set dot must **move with the index**. Both phases sample
the *same* dot and change only which OAM entry is the 33rd in-range sprite:
Comment on lines +499 to +509

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove the conflicting fixed-bracket guidance.

This section states that a fixed H-IRQ bracket is vacuous for C7.05. Later, Lines 545-549 still instruct the reader to write these rows as a bracket. Clarify that C7.05 uses the moving-index probe and state separately whether C7.06 uses a bracket.

As per path instructions, docs are the specification, so conflicting sampling instructions must be resolved.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/accuracysnes-plan.md` around lines 499 - 506, Update the C7.05 guidance
around the moving-index probe to remove or revise the later Lines 545-549
instruction that describes its rows as a fixed bracket. Explicitly state that
C7.05 samples the same dot while moving the OAM index, and separately identify
whether C7.06 uses a bracket.

Source: Path instructions


| phase | in-range sprites | 33rd is index | set dot `2i + 1` | read at ~dot 105 |
|---|---|---:|---:|---|
| A | `0..39` | 32 | 65 | **set** |
| B | `40..79` | 72 | 145 | **clear** |

Phase B additionally re-reads `$213E` in the vblank of the *same* frame and requires it set by then —
without that, "clear at dot 105" is also what a core that never sets the flag would report. The flags
clear at frame start, not on read (`Ppu::advance`), so the later read sees the same frame.

**Measure the sample dot; do not infer it.** The first revision used `HTIME = 78` on a ~22-27 dot
estimate borrowed from a different measurement. That sampled at ~170 — *past* phase B's set dot of
145 — and still passed, because the handler was then three instructions shorter and landed just
under it. Adding the `php`/`pha` that preserves the interrupted context pushed it over and the row
failed. It had been passing by a hair, and only the failure exposed it. `HTIME = 12` now samples at
95-99, ~30 dots clear on each side.

Three injections, each failing its own code: a fixed set dot fails **code 2**, never setting the flag
fails **code 1**, and setting correctly for a low index but never for a high one — which phase A
cannot catch — fails **code 3**, the guard.
Comment on lines +508 to +529

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Align all validation records with the assertions that C7.05 actually executes.

The supplied test does not measure exact set dots. It proves only that index 32 is set by approximately dot 105, while index 72 is clear at dot 105 and set by vblank. An implementation such as set_dot = index + 50 passes these assertions but violates H = 2 * index.

  • docs/accuracysnes-plan.md#L505-L519: add samples around dots 65 and 145, or narrow the coverage claim.
  • CHANGELOG.md#L14-L24: describe index-dependent movement unless exact timing is tested.
  • scripts/accuracysnes/crossval.sh#L150-L158: align the harness record with the actual cartridge assertions.

As per path instructions, AccuracySNES tests must distinguish the named behavior from alternatives, and documentation must not claim broader coverage than the corresponding assertion.

📍 Affects 3 files
  • docs/accuracysnes-plan.md#L505-L519 (this comment)
  • CHANGELOG.md#L14-L24
  • scripts/accuracysnes/crossval.sh#L150-L158
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/accuracysnes-plan.md` around lines 505 - 519, Align the C7.05 validation
documentation and harness records with the assertions actually executed: in
docs/accuracysnes-plan.md:505-519, add validation samples around dots 65 and 145
or narrow the coverage claim; in CHANGELOG.md:14-24, describe index-dependent
movement without claiming exact timing; and in
scripts/accuracysnes/crossval.sh:150-158, update the harness record to match the
cartridge assertions. Preserve the distinction between index 32 being set by
approximately dot 105 and index 72 being clear there but set by vblank.

Source: Path instructions


**Installing an IRQ handler needed new runtime machinery.** `irq_trampoline` is a bank-local
`jmp (V_IRQ_VEC)`, so that pointer can only name a bank-`$00` handler — Groups C-G, whose bodies are
relocated out of bank `$00`, could not install one at all, and no test outside `cpu.rs`/`bus.rs` ever
had. `irq_far_shim` (bank `$00`) plus the 24-bit `V_IRQ_VEC_FAR` at `$0058` fixes that for every
relocated group: point `V_IRQ_VEC` at the shim, put a 24-bit handler address in the far vector, and
the shim's `jml` reaches it while pushing nothing, so the handler still ends in a plain `rti`.

**snes9x fails this row** and is recorded as an expected divergence: it reads SET in both phases, so
its Range Over does not track the index — a scanline-granularity flag. RustySNES's position is
anchored to MesenCE on the *line* by `scripts/probes/eval-line-213e`; the *dot* is
documentation-anchored only, because the Mesen2 headless runner times out in this environment.

#### `C7.05`/`C7.06` — the probe, and the two sampling traps

An earlier revision of the table above said RustySNES sets `range_over` a line late against MesenCE.
**It does not.** Re-measured on `main`, `range_over` first reads set at **scanline 100**, which is
Expand Down
11 changes: 10 additions & 1 deletion scripts/accuracysnes/crossval.sh
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,16 @@ ran=0
# as F1.10, on the same nocash-documented behaviour. (F1.09, the busy *duration* golden, still PASSES on
# snes9x: its latch both sets and clears within the window at a slightly shorter count — 30 vs 33 — a
# recorded golden difference, not a failing test.) Region-independent.
SNES9X_KNOWN_FAILURES=12
# snes9x, +1 test (C7.05 "RangeOver dot = idx*2", fails phase B with code 2): the row asserts that
# Range Over trips at the *evaluation cycle of the 33rd in-range sprite*, H = OAM.INDEX * 2, by
# sampling the same dot with the 33rd sprite first at index 32 (set dot 65, must read SET) and then
# at index 72 (set dot 145, must read CLEAR). snes9x reads SET in both, i.e. its Range Over does not
# move with the index — a scanline-granularity flag rather than a per-sprite one, which is the
# modelling difference the row exists to detect. Documented by nocash fullsnes and the SNESdev Wiki
# (range evaluation walks OAM two cycles per sprite). RustySNES's per-dot position is anchored to
# MesenCE on the *line* by scripts/probes/eval-line-213e; the *dot* itself is documentation-anchored
# only, because the Mesen2 headless runner times out in this environment. Region-independent.
SNES9X_KNOWN_FAILURES=13

# --- snes9x, via the libretro host --------------------------------------------------------------
if [[ -f $SNES9X ]]; then
Expand Down
10 changes: 10 additions & 0 deletions tests/roms/AccuracySNES/ERROR_CODES.md
Original file line number Diff line number Diff line change
Expand Up @@ -1264,6 +1264,16 @@ Provenance: **Documented** (fullsnes and the SNESdev Wiki: sprite range evaluati
| 1 | `$02` | Range Over was already set with only 2 sprites on the scanline, so the flag is stuck and phase 2 below would report it as a count |
| 2 | `$04` | Range Over did not set with 40 sprites on one scanline at X = $100, so evaluation is skipping sprites it judges off-screen — a driver parking unused sprites to the left would see more of them survive than hardware allows |

### C7.05 — RangeOver dot = idx*2

Provenance: **Documented** (fullsnes and the SNESdev Wiki: range evaluation walks OAM two cycles per sprite, so the 33rd in-range sprite trips Range Over at H = OAM.INDEX * 2 on V = OBJ.YLOC). Kind: scored.

| Code | Byte | Meaning |
|---|---|---|
| 1 | `$02` | Range Over had not set by dot ~105 with the 33rd in-range sprite at OAM index 32 (dot 65) — the flag is later than OAM.INDEX * 2 |
| 2 | `$04` | Range Over was already set at dot ~105 with the 33rd in-range sprite at OAM index 72 (dot 145) — the set dot does not track OAM.INDEX * 2 |
| 3 | `$06` | Range Over never set at all in phase B, so its clear reading at dot ~105 proved nothing |

### C7.02 — Time Over is slivers

Provenance: **Documented** (SNESdev Wiki, Sprites; fullsnes; anomie). Kind: scored.
Expand Down
Loading
Loading