-
-
Notifications
You must be signed in to change notification settings - Fork 2
chore(release): cut v2.5.0 "Rungwork" — the 6502 rung, and the two gates it cannot reach #446
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
5d5b10c
docs(adr): ADR 0038 -- a test-only interrupt-injection API, and the
doublegate e570567
fix(cosim): length-check the power-on RAM before writing it
doublegate 3911afc
chore(release): cut v2.5.0 "Rungwork" -- the 6502 rung, and the two g…
doublegate 571cc33
docs: the v2.5.0 review round -- four findings applied, two refuted
doublegate File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| # RustyNES v2.5.0 "Rungwork" | ||
|
|
||
| **The 6502 rung, and the two gates it cannot reach.** | ||
|
|
||
| Rungwork is the scaffolding you stand on to build the next thing — and the honest name for this release, because the rung it was scoped to close does not fully close, for reasons that are structural rather than defects. | ||
|
|
||
| What is real: **nine opcode-group ROMs at 2115 records**, **4537 cycles of per-cycle bus equality**, **27,388 cycles of nestest**, and **`pc` agreeing on 100% of cycles**. What is not: nestest 0-diff over the whole run, the 5 M-cycle window, and the interrupt-injection sweep. All three are named below with why. | ||
|
|
||
| Written from public documentation only. No reference NES core was opened; none is present in the tree. | ||
|
|
||
| ## Interrupts, and the parts that actually differ | ||
|
|
||
| `nmi_n` and `irq_n`, both active low, with the distinction that matters: **/NMI is edge-sensitive and latches** — once seen it stays pending until serviced, even if the line releases — while **/IRQ is level-sensitive** and simply sampled. Implementing NMI as a level is the classic error, and it is invisible on any test whose NMI stays asserted. | ||
|
|
||
| **Delayed-`I` falls out of where the poll sits**, not from a special case. The poll is at the instruction boundary and reads `P` as it stands *before* the retiring instruction changes it, so `CLI` followed by an asserted /IRQ does not take the interrupt until one instruction later. | ||
|
|
||
| **The hijack is decided at the push, not at entry.** An NMI arriving before the vector read steals it, so a `BRK` can end up running the NMI handler having already pushed a `P` with B set. Deciding at entry makes the hijack impossible and nothing else changes — which is exactly why it needs its own test. | ||
|
|
||
| Reset does **not** latch a pending NMI from an already-low line: there was no transition, and treating the initial level as an edge fires a spurious interrupt on the first instruction of every program that ties it low. | ||
|
|
||
| ## `pc` now agrees on 100% of cycles | ||
|
|
||
| It was compared on nothing, because the two sides did not mean the same thing by it — the oracle holds the *instruction's* opcode-fetch PC across every cycle of that instruction, the DUT exposed its live register, and they agreed on **45%**: high enough to look nearly right, far too low to gate on. | ||
|
|
||
| The fix was to give the wrapper the oracle's definition, not to lower the bar. **Two corrections were needed and the first alone was not enough**: latching at the opcode fetch reached 54% and left the DUT lagging by exactly one instruction. That residual's *shape* is the diagnosis — a uniform one-record shift is a sampling phase error, never an arithmetic one, because the latch updates on the same edge as the fetch. Reporting the live PC *during* the fetch cycle and the latch everywhere else closes it: **3551 of 3551 cycles**. | ||
|
|
||
| It is latched in the wrapper, never in `cpu6502`. Same ADR 0037 argument as the rest of that file: it is a testbench view, not a register the hardware needs, and putting it in the synthesisable module is the comfortable mistake that erodes with no symptom. | ||
|
|
||
| ## Five defects, all found by the bus gate | ||
|
|
||
| None was visible to rung 1, which compares seven registers at instruction boundaries. | ||
|
|
||
| **`RTS` read the incremented address** on its final cycle; hardware reads the pulled address and increments at that cycle's end. **`AM_IZX`'s `default` arm caught cycle 1** — the operand fetch — so the core drove an effective address before the pointer byte had been read. **`AM_IZY` tested `idx_page_cross` instead of `izy_cross`**, a quantity with no meaning for a mode that adds Y to the *fetched* pointer; the first crossing case took five cycles where hardware takes six. **`AM_IZY` wrote at the unfixed address** when the index did not carry. | ||
|
|
||
| And **`build()` stamped over every ROM's interrupt vectors**, so `opgroup8`'s handlers were unreachable: the ROM assembled, every vector read `$C000`, and `BRK` "worked" by falling through to the program's first instruction. **Both sides would have agreed on that**, because both read the same wrong ROM. The fill byte is `NOP`, not an impossible value, so "was this written?" could not be answered by inspecting the byte — the assembler now returns the set of offsets the program wrote. | ||
|
|
||
| ## The two gates that do not close, and why that is written down | ||
|
|
||
| **nestest stops at a peripheral.** It reads `$2002` at cycle 27396. The oracle has a PPU and returns `$80` with vblank set; this rung's testbench is flat 64 KiB memory. **Both sides address `$2002`** — the divergence is the *data*, so it is a missing peripheral and it is rung 3 by design. The 5 M-cycle window hits the same wall much sooner. The gate is therefore bounded at 27396 and stated as a number in the Makefile, because a gate whose extent moves silently is not a gate. | ||
|
|
||
| **The interrupt-injection sweep has no oracle.** It needs NMI and IRQ asserted at chosen cycles on *both* sides, and `rustynes-core` exposes no injection API — its /IRQ comes from the APU frame counter or a mapper, its /NMI from the PPU, none of which exist at this rung. So the pins, the edge-versus-level distinction, the hijack and delayed-`I` are **implemented and not oracle-verified**. `BRK` *is* verified, because a software interrupt needs no pin and exercises the same seven-cycle sequence, the same three pushes, the same vector fetch and the same `I`-set-after-push. | ||
|
|
||
| Saying this rather than shipping the number is the point. "The 6502 rung closes" was this release's own scope, and a rung reported closed on a gate it never ran is the exact failure this programme was built to catch — the same shape as a skipped CI job satisfying a required check, or a mutation harness measuring against its own mutant. | ||
|
|
||
| ## ADR 0038, and a contract amended in both places it is stated | ||
|
|
||
| [ADR 0038](https://github.com/doublegate/RustyNES/blob/main/docs/adr/0038-cosim-interrupt-injection-api.md) records the decision to allow a **test-only, default-off** interrupt-injection API on the core, with six constraints. Two are preconditions of merging rather than follow-ups: **zero hot-path cost asserted by measurement**, and **byte-identical output verified with the feature absent *and* present-but-unused** — a distinct case, because `irq-timing-trace` selects a different per-dot loop merely by being compiled in, and that one reached the accuracy battery itself. If either fails, the decision is **void**, not "revisit", and the fallback is written down. | ||
|
|
||
| ADR 0037's hard contract and the Fabric plan both said "the emulation core is untouched", and both now point at 0038. Amending one and leaving the other is how an original claim survives its own revision. The honest restatement is "untouched in the default build, with one default-off test feature" — weaker than what eight releases of notes have said, and recorded as a cost rather than buried. | ||
|
|
||
| ## No upstream sync | ||
|
|
||
| The cadence rule is amended: the next libretro/RetroArch sync waits for the **MiSTer core to be complete**, at whatever `vX.Y.0` that lands on — not the next one on the calendar. A lagging `display_version` understates what shipped; a leading one overstates it, and the second is worse. | ||
|
|
||
| ## Verification | ||
|
|
||
| No crate under `rustynes-{cpu,ppu,apu,mappers,core}` changes, so **AccuracyCoin 141/141 (100.00%, RAM decoder) and nestest 0-diff hold by construction.** |
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.