-
-
Notifications
You must be signed in to change notification settings - Fork 2
chore(release): cut v2.4.5 "Compass" — the core reaches memory, and chooses #441
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
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,73 @@ | ||
| ## v2.4.5 "Compass" — the core reaches memory, and chooses | ||
|
|
||
| Three addressing modes, the load and store groups, and all eight branches. A compass does not tell you where you are; it tells you which way to go, which is what a branch is — and the release is also where the core first *reaches* memory rather than only shuffling registers. | ||
|
|
||
| **The emulation core is untouched.** No behaviour change to `rustynes-{cpu,ppu,apu,mappers,core}`. AccuracyCoin remains **141/141 (100.00%, RAM decoder)** and nestest 0-diff. The RTL lives in the sibling repository, pinned at **`RustyNES_MiSTer@b01a656`**. | ||
|
|
||
| --- | ||
|
|
||
| ### What matches | ||
|
|
||
| | ROM | records | covers | | ||
| |---|---|---| | ||
| | `opgroup1` | **147/147** | v2.4.4's reset and implied group — unchanged by the rewrite | | ||
| | `opgroup2` | **140/140** | every construct new in this release | | ||
|
|
||
| 287 records, compared on `cycle`, `pc`, `a`, `x`, `y`, `p`, `s`. | ||
|
|
||
| Immediate, zero page and absolute addressing; `LDA`/`LDX`/`LDY`, `STA`/`STX`/`STY`; `BPL`/`BMI`/`BVC`/`BVS`/`BCC`/`BCS`/`BNE`/`BEQ`. | ||
|
|
||
| **The ordering is not arbitrary.** Loads are what finally let a test program put an *arbitrary* value in a register — until now `A`, `X` and `Y` could only hold what power-on and increments reached from zero. Stores are the first writes this core has ever performed, so a read-back is what proves the write landed. And branches are the first opcodes to **read** a flag, which retires the dated lint waiver `p` has carried since v2.4.4. That is the outcome a dated waiver exists for: had the branches slipped, the lint would have gone on asking why. | ||
|
|
||
| ### The page-crossing branch is placed, not stumbled into | ||
|
|
||
| An `ORG` directive exists in the ROM assembler for exactly one test. At `$C0FC` the next instruction is `$C0FE`, and `+$10` lands at `$C10E` — a different page, so the taken branch costs **four** cycles instead of three. | ||
|
|
||
| An implementation that fixes the high byte without spending the cycle agrees on every register and disagrees only on `cycle`. That is why the trace compares it, and why the fix-up is written as an explicit carry/borrow rather than a 16-bit add: the hardware genuinely computes the low byte first and only then discovers the high byte was wrong, and **that discovery is the extra cycle**. | ||
|
|
||
| --- | ||
|
|
||
| ### Three things that cost real time | ||
|
|
||
| **Write intent was sampled after the clock edge.** `we` and `dout` are combinational functions of `state`/`tcyc`, and those change **on** the posedge — so reading them afterwards reads the *next* cycle's plan. Every store silently did nothing, and the symptom surfaced at a later `LDA`, several instructions after the cycle that was actually wrong. | ||
|
|
||
| **Never read RAM a test program has not written.** The oracle powers on with deterministic **seeded** work RAM; a flat-memory testbench starts at zero. A read of an untouched address diverges for a reason that has nothing to do with the CPU. Observed as the oracle returning `$36` from `$0003` where the harness returned `$00` — and the first reading of that was "the absolute store is broken". Test ROMs now write a sentinel before reading. | ||
|
|
||
| **A read-back in the same addressing mode tests round-tripping, not addressing.** `STA $10` followed by `LDA $10` is self-consistent under any address mutation: send both to `$0110` and it still passes. Two mutants — zero page to the wrong page, and absolute with its address bytes swapped — both came back **NOT CAUGHT**. Each mode is now cross-checked by a *different* mode, and the ROM says so at the site. | ||
|
|
||
| That is the third time in two releases that a mutation exposed a test which read correctly and verified nothing. It is not becoming less useful. | ||
|
|
||
| ### Mutations | ||
|
|
||
| | mutation | result | | ||
| |---|---| | ||
| | baseline / restored | MATCH | | ||
| | branch page fix-up condition inverted | **DIVERGE** | | ||
| | branch sense inverted | **DIVERGE** | | ||
| | absolute address bytes swapped | **DIVERGE** | | ||
| | `LDA` does not set N/Z | **DIVERGE** | | ||
| | zero-page store to the wrong page | **DIVERGE** | | ||
| | branch flag select off by one | **DIVERGE** | | ||
| | absolute takes 3 cycles not 4 | **DIVERGE** | | ||
|
|
||
| One had to be rewritten to be valid at all: forcing `br_fixup = 1'b0` left `br_sum[8]` and `adl[7]` unused, so Verilator refused to build it. **A mutant that does not compile is not a catch** — inverting the condition keeps both signals live and tests the same property. | ||
|
|
||
| ### Structural changes | ||
|
|
||
| **One decoder, called twice.** v2.4.4 had two duplicated `case` blocks — over `din` at fetch and over `ir` during execution — which had to agree by hand across seventeen arms. They did, and would not have kept doing so: a mode added to one and not the other is an instruction that decodes differently depending on when you look. A function returning a packed struct removes the possibility rather than documenting it. | ||
|
|
||
| At fetch time only the **operation** is consulted, not the whole struct. Taking all of it left `am` and `writes` dead, which Verilator flagged and which is correct: the addressing mode of a byte not yet latched into `IR` is not something this design has any business reading. | ||
|
|
||
| The branch condition is expressed as the 6502's own encoding — opcode bits `7:6` select the flag, bit `5` the sense — rather than eight arms, because eight arms is eight chances to invert one. | ||
|
|
||
| ### Provenance | ||
|
|
||
| Written from public documentation only — the NESdev wiki's 6502 cycle-times, instruction and addressing-mode pages, and this repository's own `docs/cpu-6502.md`. No reference NES core was opened; none is present in either tree, and CI asserts their absence. | ||
|
|
||
| ### Next | ||
|
|
||
| v2.4.6: indexed addressing (`zp,X`, `abs,X`, `abs,Y`) with its own page-cross penalty, the compare group, and `ADC`/`SBC` — the first opcodes to *consume* carry rather than only set it. | ||
|
|
||
| ### Compatibility | ||
|
|
||
| Documentation and programme-record only on the RustyNES side. No save-state, movie, netplay or public-API change in any shipped or default-build package; `.rns` and `.rnm` are unchanged. |
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.