From 234f1d2cd65479d538cc4175f63c51a8e3809ada Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Fri, 31 Jul 2026 10:26:38 -0400 Subject: [PATCH 1/3] =?UTF-8?q?perf(core):=20wire=20fast-exec=20through=20?= =?UTF-8?q?the=20scheduler=20=E2=80=94=201.53x=20on=20a=20real=20frame?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 3 of task #64, and the first number from ADR 0013's second execution mode. `System::run_until_exec` inverts who sets the pace: the CPU executes one instruction, reports what it cost in PCycles, `master_ticks` advances by that many CPU periods, and the RCP runs every one of its edges in the span that just elapsed. The CPU no longer lands on a derived edge, which is the relaxation stated plainly. ADR 0006 STILL HOLDS. `master_ticks` is still the only incremented counter and every other position is still derived from it; what changed is how far it moves per step, not who owns it. MEASURED, A-B-A in one sitting, Super Mario 64, 120 timed frames: A 100.582 / 99.847 B 64.822 / 65.321 A 99.712 / 99.598 The legs do not overlap — every A above 99.5 ms, every B below 65.4, while the four A readings span 0.99%, the ordinary within-session spread. Conservative pairing (best A over worst B): 99.598 -> 65.321 ms, **1.525x**, 10.04 -> 15.31 FPS. The return legs matter for the usual reason: two legs would have reported 1.55x from A1, and this project has been wrong that way before. Note the accurate baseline is ~99.6 ms, not the 93.06 ms recorded earlier — that figure was measured WITH `fast-scheduler` on, and its accurate pairing was 98.12 ms. Comparing a featured build against an unfeatured one is the mistake the note in docs/performance.md exists to prevent. THE DIVERGENCE IS MEASURED AND RECORDED, as ADR 0013 section 4 requires: the two modes retire 173,254,496 vs 171,471,972 instructions over the same 120 frames, +1.04%. Ledger C-16 carries the method, why it is above zero (the load-delay interlock the accurate path charges and this does not), and what would move it. Three design points, each documented where it lives: - It can land PAST `target`, by at most one instruction's cost, because a cost is only known after the instruction has run. Nothing drifts: the next call's target is absolute. - A HALTED CPU advances on RCP edges. A failed real-PIF boot checksum freezes the CPU while the RCP keeps running; with no instruction to time the advance, the loop steps to the next RCP edge. Deliberately NOT a bail-out — ADR 0011 section 6 sanctions a test-only seam only where a boundary genuinely cannot be reached, and this one can simply be handled. - `fast-exec` therefore adds NO new BailOut variant. Saying so is better than inventing an exit to justify the machinery. `run_frame` now picks one of three entry points, with fast-exec taking precedence where both features are on (ADR 0013 section 1), settled here rather than left to whichever cfg is written first. All four configurations build. `full` still excludes both, because promoting an execution mode into a shipped artifact is an ADR decision rather than a build-configuration one. CI gains five entries (core + frontend clippy, the both-features configuration, the core no_std build), because clippy runs exactly once and a cfg arm nobody compiles is a cfg arm that rots. Gates: fmt, clippy (workspace + every feature combination), test --workspace, the fast-exec CPU gate, the fast-scheduler differential gate, rustdoc, no_std, en-US, markdownlint. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 9 +++ crates/rustyn64-core/Cargo.toml | 5 ++ crates/rustyn64-core/src/lib.rs | 14 +++-- crates/rustyn64-core/src/scheduler.rs | 88 +++++++++++++++++++++++++++ crates/rustyn64-frontend/Cargo.toml | 5 ++ crates/rustyn64-frontend/src/emu.rs | 11 +++- docs/accuracy-ledger.md | 45 ++++++++++++++ docs/performance.md | 52 ++++++++++++++++ docs/scheduler.md | 28 +++++++++ 9 files changed, 250 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3e96ad35..51ee2470 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -111,6 +111,14 @@ jobs: # deliberately not, so they are graded by different predicates. - run: cargo test -p rustyn64-cpu --features fast-exec - run: cargo clippy -p rustyn64-cpu --all-targets --features fast-exec -- -D warnings + # And the core + frontend wiring. `run_frame` picks one of THREE entry + # points by `cfg`, and an arm that stops compiling is invisible to every job + # that does not enable its feature -- which is how a `#[cfg]` arm rots. + - run: cargo clippy -p rustyn64-core --all-targets --features fast-exec -- -D warnings + - run: cargo clippy -p rustyn64-frontend --all-targets --features fast-exec -- -D warnings + # Both features together is a DEFINED configuration (ADR 0013 section 1 + # settles the precedence), so it is built rather than left to chance. + - run: cargo check -p rustyn64-frontend --features fast-exec,fast-scheduler rustdoc: name: rustdoc (-D warnings) @@ -207,6 +215,7 @@ jobs: # child module of `pipeline` and uses nothing beyond what the accurate path # already does, but "uses nothing" is a claim, and this is what checks it. - run: cargo build -p rustyn64-cpu --target thumbv7em-none-eabihf --no-default-features --features fast-exec + - run: cargo build -p rustyn64-core --target thumbv7em-none-eabihf --no-default-features --features fast-exec no-commercial-roms: name: no commercial ROMs diff --git a/crates/rustyn64-core/Cargo.toml b/crates/rustyn64-core/Cargo.toml index f5bbacf9..efeab9c2 100644 --- a/crates/rustyn64-core/Cargo.toml +++ b/crates/rustyn64-core/Cargo.toml @@ -17,6 +17,11 @@ std = [] # not needed yet; that becomes due only when the fast path acquires state of its # own. fast-scheduler = [] +# ADR 0013: the instruction-granular execution mode. INDEPENDENT of +# `fast-scheduler` — neither implies the other, because one is tick-identical and +# one deliberately is not, so they are graded by different predicates (ADR 0013 +# §1). Where both are enabled, `fast-exec`'s scheduler is the one that runs. +fast-exec = ["rustyn64-cpu/fast-exec"] [dependencies] serde = { version = "1", default-features = false, features = ["derive", "alloc"] } diff --git a/crates/rustyn64-core/src/lib.rs b/crates/rustyn64-core/src/lib.rs index 5b666485..b7b446e2 100644 --- a/crates/rustyn64-core/src/lib.rs +++ b/crates/rustyn64-core/src/lib.rs @@ -19,11 +19,15 @@ pub mod boot; pub mod bus; /// The fast path's hand-off enumeration and run report (ADR 0012 §2). /// -/// Present only with the default-off `fast-scheduler` feature, so the default -/// build gains nothing at all — ADR 0011 §1 by construction, the same way -/// [`scheduler::System::run_until_fast`] is a separate entry point rather than a -/// branch. -#[cfg(feature = "fast-scheduler")] +/// Present only with a fast path compiled in — `fast-scheduler` or `fast-exec` — +/// so a default build gains nothing at all. ADR 0011 §1 by construction, the same +/// way [`scheduler::System::run_until_fast`] is a separate entry point rather than +/// a branch. +/// +/// Shared by both modes because [`fastpath::FastRunReport`] answers the same +/// question for each — did the fast path engage, and where did it hand back — even +/// though the two are graded by different predicates (ADR 0013 §1). +#[cfg(any(feature = "fast-scheduler", feature = "fast-exec"))] pub mod fastpath; pub mod scheduler; pub mod vi; diff --git a/crates/rustyn64-core/src/scheduler.rs b/crates/rustyn64-core/src/scheduler.rs index d1f322b0..46da4f65 100644 --- a/crates/rustyn64-core/src/scheduler.rs +++ b/crates/rustyn64-core/src/scheduler.rs @@ -501,6 +501,94 @@ impl System { FastRunReport { blocks, bailed } } + /// The **instruction-granular** counterpart to [`System::run_until`] + /// (ADR 0013), behind the default-off `fast-exec` feature. + /// + /// # How time advances here + /// + /// The accurate loop walks edge to edge and steps whichever domains are due. + /// This one lets the **CPU set the pace**: it executes one instruction, is told + /// what that cost in `PCycles`, advances `master_ticks` by that many CPU + /// periods, and runs the RCP over every one of its edges in the span that just + /// elapsed. The CPU therefore no longer lands on a derived edge at all — which + /// is precisely the relaxation ADR 0013 §2 authorizes and ADR 0011 §5 excludes. + /// + /// **ADR 0006 still holds.** `master_ticks` remains the only counter that is + /// ever incremented, and every other position — the RCP's edges, COP0 `Count` — + /// is still derived from it. What changed is *how far* it moves per step, not + /// who owns it. + /// + /// # It may land PAST `target`, and that is the divergence, not a defect + /// + /// A cost is only known after the instruction has run, so the last instruction + /// of a call can carry `master_ticks` beyond `target` — by at most one + /// instruction's cost. Nothing drifts: the next call's `target` is absolute, so + /// an overshoot simply means less work next time. This is the timing divergence + /// ADR 0013 §4 requires to be *measured and bounded* rather than eliminated, + /// and it is the reason this returns a report rather than nothing. + /// + /// # A halted CPU advances on RCP edges + /// + /// A failed real-PIF boot checksum freezes the CPU via NMI while the RCP keeps + /// running (`PIF-NUS.md`). With no instruction to time the advance with, this + /// steps to the next RCP edge instead. It is deliberately **not** a bail-out to + /// the accurate scheduler: forcing an ADR 0012 bail-out reason here would need + /// a fixture that can reach a checksum failure, and ADR 0011 §6 only sanctions + /// a test-only seam where a boundary *genuinely* cannot be reached — which is + /// not the case when the fast path can simply handle it. + /// + /// `fast-exec` consequently adds **no new [`BailOut`](crate::fastpath::BailOut) + /// variant**. Saying so plainly is better than inventing an exit to justify the + /// machinery; the enumeration exists so that a *real* one cannot be added + /// silently. + #[cfg(feature = "fast-exec")] + pub fn run_until_exec(&mut self, target: u64) -> crate::fastpath::FastRunReport { + use crate::fastpath::FastRunReport; + + let mut report = FastRunReport::default(); + while self.master_ticks < target { + if self.bus.boot_nmi_halt() { + let next = Self::next_edge_after(self.master_ticks, self.phases.rcp, RCP_DIVIDER); + if next > target { + break; + } + self.master_ticks = next; + self.step_rcp(); + continue; + } + + // `count_ticks` is derived from `master_ticks` and sampled BEFORE the + // instruction runs, which is the same instant the accurate path samples + // it at the CPU edge that would issue this instruction. + let count_now = self.count_ticks(); + let cost = self.cpu.step_instruction_at(&mut self.bus, count_now); + report.blocks = report.blocks.saturating_add(1); + + // One `PCycle` is `CPU_DIVIDER` master ticks (ADR 0006). `saturating_mul` + // and `saturating_add` because the product is the one arithmetic here + // that is not already bounded by `target`. + let end = self + .master_ticks + .saturating_add(u64::from(cost).saturating_mul(CPU_DIVIDER)); + + // The RCP catches up over the span the instruction consumed, on its own + // edges. CPU-before-RCP still holds: the instruction has already run. + let mut rcp = Self::next_edge_after(self.master_ticks, self.phases.rcp, RCP_DIVIDER); + while rcp <= end { + self.master_ticks = rcp; + self.step_rcp(); + rcp = Self::next_edge_after(rcp, self.phases.rcp, RCP_DIVIDER); + } + self.master_ticks = end; + } + // Only reachable through the halted branch's `break`; an executing CPU has + // already carried `master_ticks` to or past `target`. + if self.master_ticks < target { + self.master_ticks = target; + } + report + } + /// One RCP step: the RSP microcode unit, then the RDP rasterizer, then the /// AI/interface DMA progress — all on the SAME `&mut self.bus`. fn step_rcp(&mut self) { diff --git a/crates/rustyn64-frontend/Cargo.toml b/crates/rustyn64-frontend/Cargo.toml index 2683c306..d53a3038 100644 --- a/crates/rustyn64-frontend/Cargo.toml +++ b/crates/rustyn64-frontend/Cargo.toml @@ -35,6 +35,11 @@ emu-thread = [] # alternate execution mode into a shipped artifact is an ADR 0011 decision rather # than a build-configuration one. fast-scheduler = ["rustyn64-core/fast-scheduler"] +# ADR 0013's instruction-granular execution mode. Forwarded so `run_frame` can +# reach it and `examples/frame_bench.rs` can measure it; still absent from `full`, +# because promoting an alternate execution mode into a shipped artifact is an +# ADR 0013 decision rather than a build-configuration one. +fast-exec = ["rustyn64-core/fast-exec"] # Roadmap placeholders (no code yet) — kept so downstream `--features` resolve. debug-hooks = [] diff --git a/crates/rustyn64-frontend/src/emu.rs b/crates/rustyn64-frontend/src/emu.rs index 18f6fcfa..6ba152d1 100644 --- a/crates/rustyn64-frontend/src/emu.rs +++ b/crates/rustyn64-frontend/src/emu.rs @@ -263,9 +263,16 @@ impl EmuCore { // it is discarded explicitly rather than by the `#[must_use]` being absent — // an emulator that changed behavior on a diagnostic would be the ADR 0011 §6 // defect of a released path shaped by its tests. - #[cfg(feature = "fast-scheduler")] + // + // `fast-exec` takes precedence where both are enabled (ADR 0013 §1), which + // is settled here rather than left to whichever `cfg` happens to be written + // first. The three arms are mutually exclusive by construction, so exactly + // one entry point is compiled in any configuration. + #[cfg(feature = "fast-exec")] + let _ = self.system.run_until_exec(target); + #[cfg(all(feature = "fast-scheduler", not(feature = "fast-exec")))] let _ = self.system.run_until_fast(target); - #[cfg(not(feature = "fast-scheduler"))] + #[cfg(not(any(feature = "fast-scheduler", feature = "fast-exec")))] self.system.run_until(target); self.frames = self.frames.wrapping_add(1); self.produce_frame(); diff --git a/docs/accuracy-ledger.md b/docs/accuracy-ledger.md index fc38ab4b..3392d13d 100644 --- a/docs/accuracy-ledger.md +++ b/docs/accuracy-ledger.md @@ -62,6 +62,8 @@ bug, and a number that appeared without one is the failure this file exists to p | C-5 | `DIV` quotient when divisor bits 63 and 31 differ | *32x35 division* | **guessed** | needs hardware | | C-6 | Divide-by-zero `HI`/`LO` values | conventional | **guessed** | needs hardware | +| C-16 | `fast-exec` timing divergence — instructions retired over 120 frames, versus the accurate path | **+1.04%** (173,254,496 vs 171,471,972) | **measured**, Super Mario 64, `examples/frame_bench.rs`, 120 frames after the VI comes up | **the ADR 0013 §4 bound; re-measure whenever the cost model changes** | + ### C-1 — `M`, memory access time in PCycles The single most load-bearing unknown. Both documented cache-miss formulas are parameterized on @@ -271,6 +273,49 @@ not stalls), the residue invariant, determinism, and the 950-test functional sui `Failed: 0`, still 90 suite-wide, `Random` timing tests pass, runs to `xioctl(EXIT)`) are all unchanged; two unit tests that stepped fixed cycles for a cached load had their budgets widened. +### C-16 — the `fast-exec` timing divergence bound + +ADR 0013 §4 requires the divergence between the two execution modes to be +**measured, bounded, and recorded here before it ships** — not required to be zero, +because a relaxed timing model that agreed exactly would not be a relaxed timing +model. This is that measurement. + +**What is measured.** Instructions retired over an identical run: the same ROM, the +same seed, the same 120 frames after the VI comes up. Both modes present the same +number of frames, so a difference in retired instructions is a difference in how +much emulated work fell inside a frame — which is exactly the timing model's +effect, expressed in the one quantity both modes count identically. + +| mode | instructions retired, 120 frames | +| --- | --- | +| accurate (five-stage cascade) | 171,471,972 | +| `fast-exec` (instruction-granular) | 173,254,496 | +| **divergence** | **+1,782,524 — +1.04%** | + +**Method.** `cargo run --release --example frame_bench [--features fast-exec]`, +Super Mario 64, 120 timed frames after 36 warm-up frames. Both figures are stable +to the digit across repeated runs (the counter is deterministic; only the wall +clock varies), so this is a reading rather than a sample. + +**Why it is above zero, and what would move it.** The instruction-granular model +charges the documented per-class costs but not the five-stage structures the +accurate path carries — the bypass network, the load interlock, `prev_was_run`, the +flush cascade (`docs/cpu.md` §The instruction-granular path). Those are the missing +cycles. The gap is dominated by the *load-delay interlock*, which the accurate path +charges and this does not; adding it back would narrow the bound at some cost in +throughput, and that trade is not yet measured. + +**When to re-measure.** Whenever the cost model changes — a new stall, a changed +constant, a measured `M(RDRAM)` replacing the fitted cache fills (C-1). A bound +that is not re-derived after the model moves is a number describing a model that no +longer exists. + +**What this does *not* bound.** Architectural agreement, which is a separate and +stricter claim graded by `crates/rustyn64-cpu/tests/fast_exec_differential.rs` — +equality of GPRs, `HI`/`LO`, COP0, the FPRs, `FCSR`, and memory at instruction +retirement boundaries. A timing divergence of 1% is sanctioned; a divergence in +what an instruction computes is not. + ### C-2 — exception epilogue cost — **RESOLVED, and this entry was wrong** **2 PCycles, and the manual says so.** UM §4.7 (p. 114), the opening sentence of the section: diff --git a/docs/performance.md b/docs/performance.md index 918ca0d2..c43e8b0d 100644 --- a/docs/performance.md +++ b/docs/performance.md @@ -1061,6 +1061,58 @@ number* needs restating: "as fast as the cycle-accurate model can go" is reachab what the remaining work delivers; "60 FPS with the cycle-accurate model" is not, on this host, by these means. +## `fast-exec`: the instruction-granular path measures 1.53x + +The first number from ADR 0013's second execution mode, and the first result in this +document that comes from changing **what is computed** rather than when. + +**A-B-A, one sitting, `main` at the wiring commit**, Super Mario 64, 120 timed +frames after the VI comes up, `examples/frame_bench.rs`: + +| leg | build | mean frame | +| --- | --- | --- | +| A1 | accurate | 100.582 ms | +| A2 | accurate | 99.847 ms | +| B1 | `--features fast-exec` | **64.822 ms** | +| B2 | `--features fast-exec` | **65.321 ms** | +| A3 | accurate | 99.712 ms | +| A4 | accurate | 99.598 ms | + +**The legs do not overlap.** Every A is above 99.5 ms and every B below 65.4 ms, so +this is not drift: the four A readings span 0.99%, which is the ordinary +within-session spread this document records elsewhere, and the gap to B is thirty +times that. + +Quoting the **conservative** pairing — best A over worst B — as this document does +throughout: + +- **99.598 → 65.321 ms, 1.525x** +- **10.04 → 15.31 FPS** + +The return legs matter here for the reason they always do: a two-leg comparison +would have reported 1.55x from A1, and the third-time-lucky lesson in §*Ruled out* +5b is that the return leg is what tells drift from effect. + +**Why the accurate figure is ~99.6 ms and not the 93.06 ms recorded above.** That +earlier number was measured **with `fast-scheduler` enabled**; the accurate baseline +in the same pairing was 98.12 ms. So ~99.6 ms is that baseline plus ordinary +cross-session drift, not a regression. Comparing a featured build against an +unfeatured one is the mistake this note exists to prevent. + +**What it costs.** The two modes retire different instruction counts over the same +120 frames — 173,254,496 versus 171,471,972, **+1.04%** — which is the timing +divergence ADR 0013 §4 requires to be measured and bounded rather than eliminated. +It is recorded as **C-16** in `docs/accuracy-ledger.md` with its method and the +conditions that would move it. + +**What it does not do.** 60 FPS needs 16.67 ms. At 65.3 ms this is **3.92x** short, +and the ceiling arithmetic in §*The 60 FPS target is out of reach* is unchanged in +kind: the CPU pipeline was the largest single bucket and it has now been addressed, +so the remaining work is in the buckets that were never the biggest. The next +measurement to take is a fresh profile of the **fast-exec** frame — the shares +above were taken on the accurate path and no longer describe what this build +spends its time on. + ## The AI recomputed its DAC period 1.04 M times a frame `Audio::tick` opened by computing `period_ticks()` — `MASTER_HZ / sample_rate`, a **64-bit diff --git a/docs/scheduler.md b/docs/scheduler.md index 4c07b872..0ea7c040 100644 --- a/docs/scheduler.md +++ b/docs/scheduler.md @@ -334,6 +334,34 @@ the DMA'd bytes, the register writes, the eventual interrupt — must agree. The predicate is therefore anchored in **time as well as instructions**. ADR 0013 §4 is authoritative. +`System::run_until_exec` is the entry point, and it inverts who sets the pace: the +**CPU executes one instruction**, reports what it cost in `PCycles`, +`master_ticks` advances by that many CPU periods, and the RCP runs every one of its +edges in the span that just elapsed. The CPU therefore no longer lands on a derived +edge — which is the relaxation, stated plainly. + +**ADR 0006 still holds.** `master_ticks` is still the only incremented counter and +every other position is still derived from it; what changed is how far it moves per +step, not who owns it. + +Three consequences worth knowing before reading the code: + +- **It can land past `target`**, by at most one instruction's cost, because a cost + is only known after the instruction has run. Nothing drifts — the next call's + target is absolute — and this is the timing divergence ADR 0013 §4 requires to be + measured rather than eliminated. Measured: **+1.04%** over 120 frames + (`docs/accuracy-ledger.md` C-16). +- **A halted CPU advances on RCP edges.** A failed real-PIF boot checksum freezes + the CPU while the RCP keeps running; with no instruction to time the advance + with, the loop steps to the next RCP edge. Deliberately *not* a bail-out: + ADR 0011 §6 sanctions a test-only seam only where a boundary genuinely cannot be + reached, and this one can simply be handled. +- **`fast-exec` therefore adds no new `BailOut` variant.** Saying so is better than + inventing an exit to justify the machinery; the enumeration exists so a *real* + one cannot be added silently. + +Measured **1.53x** on a real frame (`docs/performance.md`). + This is **`fast-exec` policy, not a hardware claim.** The VR4300 has one timeline, so the hardware has no opinion about how two emulated modes should be compared; there is no manual section to cite, and citing one would make a policy read as a From 10dadcddf8d8cdc0e5353fef20098205994ab1c6 Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Fri, 31 Jul 2026 10:40:14 -0400 Subject: [PATCH 2/3] fix(core): four review findings, and a test that measured a derived accessor All four adopted; the third is the one worth reading. 1. CI LINTED THE CORE WIRING WITHOUT EXECUTING IT. `run_until_exec` had no test at all -- the CPU crate's differential gate cannot reach it. Added crates/rustyn64-core/tests/fast_exec_scheduler.rs (5 tests: engagement, the bounded overshoot swept across an RCP period, the no-op, the RCP catch-up, and both modes honoring the same target contract) plus the missing `cargo test -p rustyn64-core --features fast-exec` CI entry. 2. `FastRunReport::blocks` MEANT TWO DIFFERENT THINGS. `run_until_fast` counts whole edge periods; `run_until_exec` counted instructions. Same public field, two units, and a consumer could not read it consistently. Renamed to `work_units`, with a table naming the unit per mode and an explicit "do not compare across modes" -- it answers ADR 0012's *did it engage*, for which any positive count means the same thing, and a magnitude comparison between two units means nothing. 3. THE RCP CATCH-UP TEST WAS VACUOUS, and mutation is what showed it. The first version asserted `System::rcp_cycles` advanced. Deleting the entire RCP catch-up loop left it GREEN -- because `rcp_cycles` is a DERIVED ACCESSOR off `master_ticks` (ADR 0006's whole point), so it advances whether or not a single RCP step ran. The witness has to be RETAINED state. `VI_V_CURRENT` is incremented by `Vi::tick`, which only `step_rcp` calls, so it stays put if the RCP never steps. The mutation now fails with exactly that message. The general lesson, recorded in the test: in a codebase that derives every position from one counter, most of the obvious progress indicators cannot witness that work happened. Ask what is STORED, not what is REPORTED. 4. RCP EDGE-SEARCH OVERFLOW. `saturating_add` could put `end` at u64::MAX WITHOUT the machine ever having run there, and `next_edge_after` then evaluates `tick + 1` at u64::MAX -- a panic in debug, and in release a wrap to a low tick that keeps `rcp <= end` true forever. That is the difference from the accurate loop, whose top-of-range is unreachable because reaching it means emulating three thousand years. Now `checked_mul` + `checked_add`, and overflow ends the run. Also: docs/scheduler.md's comparison table claimed `fast-exec` amends ADR 0006 with per-domain deficit counters. The implementation has none -- that is the deficit-counter scheduler, still ahead. Corrected to say ADR 0006 is unchanged AS IMPLEMENTED, with the authorization falling due when the counters arrive. The halted-CPU branch is still untested, and the test file records why rather than leaving a hole: `boot_nmi_halt` is latched only by a real-PIF boot with a failing IPL2 checksum, which needs images that are never committed. A test-only seam on `Bus` for one three-line branch is not what ADR 0011 section 6 sanctions. Gates: fmt, clippy (workspace + every feature), test --workspace, both feature test suites, the CPU gate, rustdoc, no_std, en-US, markdownlint. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 1 + crates/rustyn64-core/src/fastpath.rs | 54 +++-- crates/rustyn64-core/src/scheduler.rs | 39 +++- .../tests/fast_exec_scheduler.rs | 188 ++++++++++++++++++ .../tests/fast_scheduler_differential.rs | 10 +- docs/scheduler.md | 2 +- 6 files changed, 260 insertions(+), 34 deletions(-) create mode 100644 crates/rustyn64-core/tests/fast_exec_scheduler.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 51ee2470..b5274545 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -114,6 +114,7 @@ jobs: # And the core + frontend wiring. `run_frame` picks one of THREE entry # points by `cfg`, and an arm that stops compiling is invisible to every job # that does not enable its feature -- which is how a `#[cfg]` arm rots. + - run: cargo test -p rustyn64-core --features fast-exec - run: cargo clippy -p rustyn64-core --all-targets --features fast-exec -- -D warnings - run: cargo clippy -p rustyn64-frontend --all-targets --features fast-exec -- -D warnings # Both features together is a DEFINED configuration (ADR 0013 section 1 diff --git a/crates/rustyn64-core/src/fastpath.rs b/crates/rustyn64-core/src/fastpath.rs index 5870d579..607a55ab 100644 --- a/crates/rustyn64-core/src/fastpath.rs +++ b/crates/rustyn64-core/src/fastpath.rs @@ -173,29 +173,41 @@ impl BailOutSet { /// *"the fast path never engaged"* and *"no bail-out boundary was reached"* — and /// both would otherwise look exactly like success. /// -/// `blocks` is the engagement half: a fast path that quietly deferred everything to -/// the accurate scheduler would agree with it perfectly and prove nothing, which is -/// literally what #224 shipped on purpose while the gate was being built. It is a -/// count and not a flag because "engaged once over a whole suite" and "engaged -/// throughout" are worth telling apart in a failure message. +/// `work_units` is the engagement half: a fast path that quietly deferred +/// everything to the accurate scheduler would agree with it perfectly and prove +/// nothing, which is literally what #224 shipped on purpose while the gate was +/// being built. It is a count and not a flag because "engaged once over a whole +/// suite" and "engaged throughout" are worth telling apart in a failure message. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] #[must_use = "the run report is ADR 0012's completion witness; a caller that drops \ it should say so explicitly with `let _ =`"] pub struct FastRunReport { - /// Whole edge periods the block executor ran itself. - pub blocks: u64, + /// How much work the fast path did itself — **in a unit the mode defines**. + /// + /// | mode | one unit is | + /// | --- | --- | + /// | `fast-scheduler` (`System::run_until_fast`) | one whole edge period (`EDGE_PERIOD` ticks) | + /// | `fast-exec` (`System::run_until_exec`) | one CPU instruction | + /// + /// **Do not compare it across modes**, and do not read it as a metric. It + /// answers ADR 0012 §2's question — *did the fast path engage* — for which any + /// positive count means the same thing either way, and a comparison of + /// magnitudes between two different units means nothing at all. Named + /// `work_units` rather than `blocks` precisely because "blocks" reads as a + /// single unit and it is not one; raised in review. + pub work_units: u64, /// Reasons the accurate scheduler was handed a stretch. pub bailed: BailOutSet, } impl FastRunReport { - /// Did the block executor run at all? + /// Did the fast path do any work itself? #[must_use] pub const fn engaged(self) -> bool { - self.blocks > 0 + self.work_units > 0 } - /// Accumulate another call's report: blocks **add**, reasons **union**. + /// Accumulate another call's report: work units **add**, reasons **union**. /// /// The rule lives here rather than at each call site because the two halves /// fail differently. A caller that adds the counts and forgets the union still @@ -203,16 +215,17 @@ impl FastRunReport { /// is the one thing ADR 0012 §2's witness exists to catch, so a bug in the /// accumulation would disable the check rather than trip it. /// - /// `saturating_add` because a block count is a diagnostic: a wrapped total - /// could read as zero and turn "engaged constantly" into "never engaged". It - /// cannot be reached in practice — `u64::MAX` blocks is 6 x 2^64 master - /// ticks — but saturating is the failure that stays interpretable. + /// `saturating_add` because the count is a diagnostic: a wrapped total could + /// read as zero and turn "engaged constantly" into "never engaged". It cannot + /// be reached in practice — `u64::MAX` units is more emulated time than the + /// tick counter itself can express — but saturating is the failure that stays + /// interpretable. /// /// (No `#[must_use]` here: the returned type already carries one, and clippy's /// `double_must_use` rejects a second without a distinct message.) pub const fn merge(self, other: Self) -> Self { Self { - blocks: self.blocks.saturating_add(other.blocks), + work_units: self.work_units.saturating_add(other.work_units), bailed: self.bailed.union(other.bailed), } } @@ -268,19 +281,22 @@ mod tests { } } - /// A report with no blocks has not engaged, whatever else it says. + /// A report with no work units has not engaged, whatever else it says. #[test] - fn engagement_is_about_blocks_not_bailouts() { + fn engagement_is_about_work_units_not_bailouts() { let mut bailed = BailOutSet::new(); bailed.insert(BailOut::PartialPeriodTail); - let deferred = FastRunReport { blocks: 0, bailed }; + let deferred = FastRunReport { + work_units: 0, + bailed, + }; assert!( !deferred.engaged(), "a run that executed no block has not engaged, even having handed off" ); assert!( FastRunReport { - blocks: 1, + work_units: 1, ..deferred } .engaged() diff --git a/crates/rustyn64-core/src/scheduler.rs b/crates/rustyn64-core/src/scheduler.rs index 46da4f65..db44e7b4 100644 --- a/crates/rustyn64-core/src/scheduler.rs +++ b/crates/rustyn64-core/src/scheduler.rs @@ -498,7 +498,10 @@ impl System { } self.run_until(target); - FastRunReport { blocks, bailed } + FastRunReport { + work_units: blocks, + bailed, + } } /// The **instruction-granular** counterpart to [`System::run_until`] @@ -562,14 +565,29 @@ impl System { // it at the CPU edge that would issue this instruction. let count_now = self.count_ticks(); let cost = self.cpu.step_instruction_at(&mut self.bus, count_now); - report.blocks = report.blocks.saturating_add(1); - - // One `PCycle` is `CPU_DIVIDER` master ticks (ADR 0006). `saturating_mul` - // and `saturating_add` because the product is the one arithmetic here - // that is not already bounded by `target`. - let end = self - .master_ticks - .saturating_add(u64::from(cost).saturating_mul(CPU_DIVIDER)); + report.work_units = report.work_units.saturating_add(1); + + // One `PCycle` is `CPU_DIVIDER` master ticks (ADR 0006). + // + // **`checked`, not `saturating`.** Saturating was the first version and + // it is a real hazard here rather than a theoretical one: it can put + // `end` at `u64::MAX` *without the machine ever having run there*, and + // `next_edge_after` then evaluates `tick + 1` at `u64::MAX` — a panic in + // debug, and in release a wrap to a low tick that keeps `rcp <= end` + // true forever. That is the difference between this and the accurate + // loop, whose top-of-range is unreachable because reaching it means + // emulating three thousand years (the note in the fast-scheduler gate). + // Raised in review. + // + // Overflowing means the timeline is exhausted, so the run ends; the + // landing below carries `master_ticks` to `target` if it is not already + // past it. + let Some(end) = u64::from(cost) + .checked_mul(CPU_DIVIDER) + .and_then(|span| self.master_ticks.checked_add(span)) + else { + break; + }; // The RCP catches up over the span the instruction consumed, on its own // edges. CPU-before-RCP still holds: the instruction has already run. @@ -577,6 +595,9 @@ impl System { while rcp <= end { self.master_ticks = rcp; self.step_rcp(); + // `end` is a real tick because it came from `checked_add` above, so + // this cannot be asked for an edge past `u64::MAX`: the loop + // condition fails at or before it. rcp = Self::next_edge_after(rcp, self.phases.rcp, RCP_DIVIDER); } self.master_ticks = end; diff --git a/crates/rustyn64-core/tests/fast_exec_scheduler.rs b/crates/rustyn64-core/tests/fast_exec_scheduler.rs new file mode 100644 index 00000000..c38cd879 --- /dev/null +++ b/crates/rustyn64-core/tests/fast_exec_scheduler.rs @@ -0,0 +1,188 @@ +//! Scheduler-level tests for `fast-exec` (ADR 0013). +//! +//! The CPU crate's `fast_exec_differential` grades what an instruction *computes*. +//! Nothing there can reach `System::run_until_exec`, which is where the mode's +//! **scheduling** lives: the RCP catching up over an elapsed span, the overshoot +//! past `target`, and the halted-CPU branch. This file is that half — added after +//! review pointed out that CI linted the wiring without executing it. +//! +//! Runs only with `--features fast-exec`; the file is empty otherwise. + +#![cfg(feature = "fast-exec")] + +use rustyn64_core::scheduler::{CPU_DIVIDER, RCP_DIVIDER, System}; +use rustyn64_core::vi::{VI_H_TOTAL, VI_V_CURRENT, VI_V_TOTAL}; + +/// A seed with no special structure; the phase alignment it derives is what makes +/// the RCP-edge arithmetic below a real test rather than one aligned case. +const SEED: u64 = 0x5265_616C_6974_7921; + +/// The whole point of the mode: it advances the machine without walking edges. +#[test] +fn it_engages_and_reaches_the_target() { + const TARGET: u64 = 200_000; + + let mut sys = System::new(SEED); + let report = sys.run_until_exec(TARGET); + + assert!( + report.engaged(), + "the fast path executed no instruction, so it deferred the whole run" + ); + assert!( + sys.master_ticks() >= TARGET, + "the run finished at {} but was asked for {TARGET}", + sys.master_ticks() + ); + assert!( + sys.cpu.retired > 0, + "no instruction retired — the run advanced the clock without executing" + ); +} + +/// **The overshoot is bounded**, which is the claim `run_until_exec`'s doc comment +/// makes and the reason ADR 0013 §4 asks for a measured divergence rather than +/// none. +/// +/// A cost is only known after the instruction has run, so the last instruction of a +/// call can carry `master_ticks` past `target`. What must not happen is an +/// *unbounded* overshoot: that would mean the loop ran on after the target. +#[test] +fn the_overshoot_is_at_most_one_instructions_cost() { + /// Comfortably above the most expensive single instruction — `DDIV` 69 plus a + /// cache fill and the exception epilogue is under 200 `PCycles`. Expressed in + /// master ticks, hence the divider. + const MAX_INSTRUCTION_TICKS: u64 = 200 * CPU_DIVIDER; + + // Sweep targets across a whole RCP period so no single alignment can pass by + // luck; an off-by-one in the RCP catch-up shows at one offset and not another. + for offset in 0..RCP_DIVIDER * 4 { + let target = 50_000 + offset; + let mut sys = System::new(SEED); + let _ = sys.run_until_exec(target); + let landed = sys.master_ticks(); + assert!( + landed >= target, + "offset {offset}: landed short at {landed}" + ); + assert!( + landed - target < MAX_INSTRUCTION_TICKS, + "offset {offset}: overshot by {} ticks, more than one instruction can \ + cost — the loop kept running past the target", + landed - target + ); + } +} + +/// A target at or before the current tick must do nothing at all. +/// +/// The mirror of `fast_scheduler_differential`'s no-op test, and it matters more +/// here: this loop's exit condition is `master_ticks < target` rather than an edge +/// search, so a sign error would execute one whole instruction. +#[test] +fn a_reached_target_is_a_no_op() { + let mut sys = System::new(SEED); + let _ = sys.run_until_exec(10_000); + let (ticks, retired) = (sys.master_ticks(), sys.cpu.retired); + + for target in [ticks, ticks - 1, 0] { + let report = sys.run_until_exec(target); + assert!( + !report.engaged(), + "a reached target ({target}) executed an instruction" + ); + } + assert_eq!( + sys.master_ticks(), + ticks, + "the clock moved for a reached target" + ); + assert_eq!( + sys.cpu.retired, retired, + "work retired for a reached target" + ); +} + +/// **The RCP runs over the span the CPU consumed**, which is the half of this mode +/// that is easiest to omit and hardest to notice: a fast path that advanced +/// `master_ticks` and never stepped the RCP would still retire instructions, still +/// reach the target, and still pass every other test in this file. +/// +/// # The witness has to be RETAINED state, not a derived position +/// +/// The obvious assertion — that `System::rcp_cycles` advanced — is **vacuous**, and +/// mutation-checking is what showed it: deleting the RCP catch-up loop entirely +/// left that version green. `rcp_cycles` is a *derived accessor* off `master_ticks` +/// (ADR 0006's whole point), so it advances whether or not a single RCP step ran. +/// +/// `VI_V_CURRENT` is the opposite: the VI's half-line counter is **incremented** by +/// `Vi::tick`, which only `step_rcp` calls. If the RCP never steps it stays where +/// it started, and no amount of clock advancing moves it. +/// +/// The general lesson is worth the paragraph: in a codebase that derives every +/// position from one counter, *most* of the obvious progress indicators cannot +/// witness that work happened. Ask what is stored, not what is reported. +#[test] +fn the_rcp_actually_steps_over_the_elapsed_span() { + const TARGET: u64 = 400_000; + + let mut sys = System::new(SEED); + // The VI needs a programmed frame geometry before its scan advances; a bare + // power-on VI has `VI_V_TOTAL == 0` and holds. These are the NTSC defaults the + // scan-out tests use. + sys.bus.vi.write(VI_V_TOTAL, 524); + sys.bus.vi.write(VI_H_TOTAL, 0x0000_0C15); + let before = sys.bus.vi.read(VI_V_CURRENT); + + let _ = sys.run_until_exec(TARGET); + + assert_ne!( + sys.bus.vi.read(VI_V_CURRENT), + before, + "the VI half-line never moved over {} ticks, so `step_rcp` was never \ + called: the CPU ran alone and every other assertion here is about a \ + machine with no RCP in it", + sys.master_ticks() + ); +} + +/// The accurate and fast paths must reach the **same target** from the same seed, +/// even though they take different amounts of emulated work to get there. +/// +/// Not an equality gate on state — ADR 0013 §4 explicitly does not ask for that +/// here — but it does pin that both modes *terminate on the same contract*, which +/// is what every caller of `run_until` relies on. +#[test] +fn both_modes_honor_the_same_target_contract() { + const TARGET: u64 = 120_000; + + let mut accurate = System::new(SEED); + accurate.run_until(TARGET); + assert_eq!( + accurate.master_ticks(), + TARGET, + "the accurate path lands exactly on the target" + ); + + let mut fast = System::new(SEED); + let _ = fast.run_until_exec(TARGET); + assert!( + fast.master_ticks() >= TARGET, + "the fast path reaches the target (and may pass it, by design)" + ); +} + +// **The halted-CPU branch is NOT tested here, and this records why rather than +// leaving a hole to be discovered.** +// +// `Bus::boot_nmi_halt` is latched only by `pif_boot_command_if_cmd`, on a real-PIF +// boot whose IPL2 checksum verify fails. Reaching it needs a PIF ROM and a +// deliberately corrupted image; `crates/rustyn64-test-harness/tests/commercial_boot.rs` +// is where that is exercised, and commercial/PIF images are never committed. +// +// A `#[cfg(test)]` setter would not help — this is an integration test, so it +// compiles against the crate with `cfg(test)` false — and a test-only *feature* on +// `Bus` would put a seam in the shipped path for one branch. ADR 0011 §6 permits +// that only where a boundary genuinely cannot be reached any other way, and the +// branch is three lines whose failure mode (a halted CPU stops the clock) is loud +// rather than silent. The honest state is: covered by inspection, not by a test. diff --git a/crates/rustyn64-core/tests/fast_scheduler_differential.rs b/crates/rustyn64-core/tests/fast_scheduler_differential.rs index ec50b13e..a7879bd1 100644 --- a/crates/rustyn64-core/tests/fast_scheduler_differential.rs +++ b/crates/rustyn64-core/tests/fast_scheduler_differential.rs @@ -233,7 +233,7 @@ fn the_fast_path_agrees_at_every_phase_alignment() { let mut accurate = System::new(seed); let mut fast = System::new(seed); accurate.run_until(target); - blocks += fast.run_until_fast(target).blocks; + blocks += fast.run_until_fast(target).work_units; assert!( accurate.cpu.retired > 0, @@ -711,8 +711,8 @@ fn the_gate_witnesses_its_own_completion() { let report = run_fixture(fixture); total = total.merge(report); println!( - "fixture {}: {} blocks, reasons {:?}", - fixture.name, report.blocks, report.bailed + "fixture {}: {} work units, reasons {:?}", + fixture.name, report.work_units, report.bailed ); assert!( started.elapsed() < SUITE_TIMEOUT, @@ -770,10 +770,10 @@ fn the_gate_witnesses_its_own_completion() { ); println!( - "GATE COMPLETE: {} fixtures, {} blocks executed, {}/{} bail-out \ + "GATE COMPLETE: {} fixtures, {} work units executed, {}/{} bail-out \ reasons covered", selected.len(), - total.blocks, + total.work_units, BailOut::ALL.len(), BailOut::ALL.len() ); diff --git a/docs/scheduler.md b/docs/scheduler.md index 0ea7c040..7732c81c 100644 --- a/docs/scheduler.md +++ b/docs/scheduler.md @@ -313,7 +313,7 @@ with more in it, and the two features are independent: | relation to the accurate run | **tick-identical** — same edges, same order, same `master_ticks` | timing deliberately diverges | | gate predicate | whole serialized state, every tick | architectural state at retirement boundaries, **minus the timing-derived carve-out**; timing divergence measured and bounded | | `master_ticks` equality | asserted | **not** asserted — it is the quantity being relaxed | -| ADR 0006 | unchanged | amended for that mode only (per-domain deficit counters) | +| ADR 0006 | unchanged | **unchanged as implemented.** ADR 0013 *authorizes* per-domain deficit counters for this mode; the implementation has none, and `master_ticks` is still the only incremented counter. The authorization falls due with the deficit-counter scheduler, not before. | Where both features are enabled, `fast-exec`'s scheduler is the one that runs (ADR 0013 §1). The whole-state tests above keep their stricter predicate: they grade From c6727e2a7f274b3018b98aa976e30657b681a1ae Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Fri, 31 Jul 2026 10:44:41 -0400 Subject: [PATCH 3/3] fix(core): enforce cost > 0 where the loop's termination depends on it Antigravity flagged a hang if `step_instruction_at` ever returns 0: `end` stays at `master_ticks`, no RCP edge steps, and the outer loop spins. It is NOT reachable today -- `Pipeline::step_instruction` charges one PCycle to issue before adding anything -- but that guarantee lives in ANOTHER CRATE, and a reader of this loop cannot see it. The hazard is the remoteness, not the absence, and the finding is right on those grounds. A `debug_assert` is the real check. The `max(1)` beside it is a floor so a release build makes progress rather than hanging; it is deliberately not a fix, since a zero cost is a defect either way, but running one PCycle fast is a far better way to report a defect than a hang. Also adds the `cargo test -p rustyn64-frontend --features fast-exec` CI entry. The core one and the FastRunReport unit documentation were already in 10dadcd, which the review predates. Gates: fmt, clippy (workspace + every feature), test --workspace, both feature suites, the frontend feature suite, rustdoc, en-US, markdownlint. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 1 + crates/rustyn64-core/src/scheduler.rs | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b5274545..0f0eeeea 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -119,6 +119,7 @@ jobs: - run: cargo clippy -p rustyn64-frontend --all-targets --features fast-exec -- -D warnings # Both features together is a DEFINED configuration (ADR 0013 section 1 # settles the precedence), so it is built rather than left to chance. + - run: cargo test -p rustyn64-frontend --features fast-exec - run: cargo check -p rustyn64-frontend --features fast-exec,fast-scheduler rustdoc: diff --git a/crates/rustyn64-core/src/scheduler.rs b/crates/rustyn64-core/src/scheduler.rs index db44e7b4..f36cd245 100644 --- a/crates/rustyn64-core/src/scheduler.rs +++ b/crates/rustyn64-core/src/scheduler.rs @@ -582,7 +582,23 @@ impl System { // Overflowing means the timeline is exhausted, so the run ends; the // landing below carries `master_ticks` to `target` if it is not already // past it. - let Some(end) = u64::from(cost) + // **This loop's termination depends on `cost > 0`, and nothing here + // enforced it.** `Pipeline::step_instruction` charges one `PCycle` to + // issue before adding anything, so zero is not reachable today — but + // that is an invariant held in another crate, and a reader of *this* + // loop cannot see it. A zero would leave `end == master_ticks`, step no + // RCP edges, and spin forever. Raised in review as blocking, correctly: + // the hazard is that the guarantee is remote, not that it is absent. + // + // The `debug_assert` is the real check; the `max(1)` is a floor so a + // release build makes progress rather than hanging. It is deliberately + // NOT a fix — a zero cost would be a defect either way — but running + // one `PCycle` fast is a far better way to report one than a hang. + debug_assert!( + cost > 0, + "an instruction cost 0 PCycles, which cannot happen" + ); + let Some(end) = u64::from(cost.max(1)) .checked_mul(CPU_DIVIDER) .and_then(|span| self.master_ticks.checked_add(span)) else {