Skip to content

feat(harness): author I-cache-fill hardware timing ROM - #140

Merged
doublegate merged 2 commits into
mainfrom
feat/icache-timing-rom
Jul 25, 2026
Merged

feat(harness): author I-cache-fill hardware timing ROM#140
doublegate merged 2 commits into
mainfrom
feat/icache-timing-rom

Conversation

@doublegate

Copy link
Copy Markdown
Owner

Motivation

The VR4300 I-cache line-fill cost (M_ICACHE_FILL, ledger C-1) is
currently a value fitted from ares/cen64 — no hardware cached-miss timing
oracle exists in the emulation community's corpus (PeterLemon times only ALU
ops, n64-systemtest's Timing set is two COP0 Random tests, N64brew/copetti
give only a cold-access range). The D-cache companion ROM (mrdram_timing.z64)
already makes the D-cache measurement one console-run away; this adds the
I-cache counterpart so M_ICACHE_FILL can likewise be replaced with a real
measurement when hardware is available.

Changes

  • tools/mrdram-timing-rom/icache_timing.asm + .z64 — a bare-metal ROM
    (bass, ARM9 fork; blank IPL3, no Nintendo code) that runs a straight-line
    block of N=8192 addius (32 KiB > the 16 KiB I-cache), so every 32-byte
    fetch line misses. Times it with COP0 Count and reports
    fill = (delta*2 - N)/(N/8) after subtracting the verified 1-PClock base.
    Results go to fixed RDRAM words (phys 0x10000, past the code) and the
    ISViewer channel — the same output path as the D-cache ROM.
  • build.sh now assembles both ROMs; README documents the I-cache
    variant (formula, output, hardware steps).
  • crates/rustyn64-test-harness/tests/icache_timing_rom.rs — boots the ROM
    through load_direct and asserts it reads back the charged M_ICACHE_FILL
    (measures 46.09 in-emulator, the charged 46): proof the measurement
    path is correct end-to-end, and a regression guard tying the ROM to the
    constant.
  • .gitignore re-includes icache_timing.z64 by exact filename;
    ledger C-1 and CHANGELOG note both authored hardware ROMs.

No emulated behaviour changes — this is instrumentation.

Gates run locally

  • cargo fmt --all --check — clean
  • cargo clippy -p rustyn64-test-harness --all-targets -- -D warnings — clean
  • cargo test -p rustyn64-test-harness --release --test icache_timing_rom --test mrdram_timing_rom — green
  • pre-commit run markdownlint on the touched docs — passed
  • scripts/check_no_roms.sh (with the ROM staged) — clean (allowlisted)

🤖 Generated with Claude Code

Companion to the D-cache M(RDRAM) timing ROM (tools/mrdram-timing-rom/): a
bare-metal N64 ROM that measures the VR4300 I-cache line-fill cost on real
hardware. It runs a straight-line block of N=8192 addiu instructions (32 KiB,
larger than the 16 KiB I-cache) so every 32-byte fetch line misses, times it
with COP0 Count, and reports fill = (delta*2 - N)/(N/8) after subtracting the
verified 1-PClock-per-instruction base. Results go to fixed RDRAM words (phys
0x10000, past the code block) and the ISViewer text channel for a flashcart to
read -- the same output path as the D-cache ROM.

This makes the eventual hardware measurement of M_ICACHE_FILL one console-run
away, replacing the value currently FITTED from ares/cen64 (ledger C-1).

- tools/mrdram-timing-rom/icache_timing.asm + .z64 (bass, ARM9 fork; blank
  IPL3, no Nintendo code; MIT OR Apache-2.0).
- build.sh now assembles both ROMs; README documents the I-cache variant.
- crates/rustyn64-test-harness/tests/icache_timing_rom.rs boots the ROM through
  load_direct and asserts it reads back the charged M_ICACHE_FILL (measures
  46.09 in-emulator, the charged 46) -- proof the measurement path is correct
  end-to-end and a guard tying the ROM to the constant.
- .gitignore re-includes icache_timing.z64 by exact filename; ledger C-1 and
  CHANGELOG note both authored hardware ROMs.

Gates: cargo fmt --check, clippy -D warnings (test-harness), both ROM runners
green, markdownlint on the touched docs, check_no_roms clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@doublegate, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 1 minute

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d3b0fbce-d431-44f3-89e6-ae2f46714ce6

📥 Commits

Reviewing files that changed from the base of the PR and between 4c54ea7 and 86e5236.

📒 Files selected for processing (8)
  • .gitignore
  • CHANGELOG.md
  • crates/rustyn64-test-harness/tests/icache_timing_rom.rs
  • docs/accuracy-ledger.md
  • tools/mrdram-timing-rom/README.md
  • tools/mrdram-timing-rom/build.sh
  • tools/mrdram-timing-rom/icache_timing.asm
  • tools/mrdram-timing-rom/icache_timing.z64

Comment @coderabbitai help to get the list of available commands.

Adopt Antigravity review suggestion #2: on a hung ROM or wrong entry point the
sentinel-poll loop exits on the step cap, after which assert_eq!(n, 8192) fails
with an opaque mismatch. An explicit non-timeout assert makes that failure mode
self-explanatory.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@doublegate

Copy link
Copy Markdown
Owner Author

Thanks — adjudicated each point:

Suggestion 1 (build.sh set -e) — Rejected: set -e is already present (build.sh:19), directly above the $BASS invocations, so a failed assemble does abort before the success echo. The suggestion appears to have read the diff hunk without the surrounding context.

Suggestion 2 (assert non-timeout in the runner) — Adopted in 86e5236. Added an explicit assert!(steps < 20_000_000, …) after the sentinel-poll loop so a hung ROM / wrong entry point reports a self-explanatory failure instead of the opaque assert_eq!(n, 8192) mismatch.

Nitpick (sp initialized but unused) — Kept, deliberately: this mirrors the D-cache companion ROM (mrdram_timing.asm:81 does the identical lui sp, 0x8020). It is conventional boot hygiene (a defined stack before any jal), harmless, and parity between the two ROMs is worth more here than shaving one instruction.

@github-actions

Copy link
Copy Markdown

Antigravity review (Gemini via Ultra)

This pull request adds an authored bare-metal I-cache timing ROM (icache_timing.z64), its assembly source, documentation updates, and an integration test runner to measure and guard the charged I-cache fill constant.

Blocking issues

None found.

Suggestions

  • icache_timing_rom.rs:43: The step cap 20_000_000 is duplicated in the while condition and the assert! assertion message. Define a const MAX_STEPS: u64 = 20_000_000; constant to prevent magic number duplication.
  • icache_timing_rom.rs:27-30: word() accesses sys.bus.rdram using raw indexing r[phys..phys+4]. Using checked indexing or .get() would prevent a potential panic if RDRAM configuration or test bus sizing changes.
  • icache_timing.asm:75-80: mtc0 r0, COUNT is executed prior to jal StraightBlock, meaning Count includes execution overhead for jal, jr ra, and their delay slots. Moving the COP0 counter read/write instructions directly inside the target block would eliminate the ~0.09 PClock measurement residual noted in the documentation.

Nitpicks

  • icache_timing_rom.rs:61: The calculation uses inline literals 2.0 (PClock multiplier) and 8.0 (instructions per 32-byte line); consider naming these constants for clarity.
  • .gitignore:113: Committing compiled binary blobs (icache_timing.z64) directly into the repository increases repository footprint over time.

Automated first-pass review by agy on a self-hosted runner -- not a human review.

@doublegate
doublegate merged commit 6650779 into main Jul 25, 2026
11 checks passed
@doublegate
doublegate deleted the feat/icache-timing-rom branch July 25, 2026 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant