Repository navigation
Add arczi84's NFS3 floating-point kernels - #5
Merged
Merged
Conversation
memcpy-var-small and the memmove rows overflowed 12 columns; MEAN MS sat one column right of its values. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
libnix checked for it in its stdio; newlib, the C library since 0e07c1d, does not. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The skipped rows stay out of every geomean; the chart labels a missing one "not built" rather than failing the column. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Two floating-point benchmarks from his Need for Speed III port, C++ and 0BSD, so only the gcc build has them; the FPU runs them in single precision, as the game does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The compiler comparison's geomean leaves the C++ rows out; the LHa rows move a few ms with the prototypes of 80f469d. Measured with volamos 0.9.0, which matches 0.8.0 cycle for cycle. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟡 Changes recommended
Skipped benchmarks with checksum mismatches incorrectly suppress the chart’s otherwise valid geomean.
1 open finding
What changed in this PR
Adds two gcc-only NFS3 floating-point benchmarks and integrates them into builds, reporting, and documentation.
Changes:
- Adds projection and clipping kernels with benchmark harnesses.
- Extends build and results tooling for C++ and selective geomeans.
- Refreshes benchmark results and adds Ctrl-C handling.
| File | Description |
|---|---|
tools/results-table.py |
Adds geomean exclusions. |
tools/results-chart.py |
Charts excluded benchmarks. |
third_party/nfs3/source.sha256 |
Records upstream source hash. |
third_party/nfs3/README.md |
Documents kernel provenance. |
third_party/nfs3/nfs3.h |
Declares C-compatible entry points. |
third_party/nfs3/LICENSE |
Adds the 0BSD license. |
third_party/nfs3/kernel.cpp |
Implements both floating-point kernels. |
third_party/LICENSE.md |
Registers the NFS3 license. |
src/main.c |
Registers benchmarks and handles Ctrl-C. |
src/bench.h |
Exposes benchmark descriptors. |
results/opt-levels-size-040.svg |
Updates code-size results. |
results/opt-levels-040.svg |
Updates optimization charts. |
results/compilers-040.svg |
Updates compiler comparison chart. |
RESULTS.md |
Documents refreshed measurements. |
README.md |
Lists the new benchmarks. |
Makefile |
Adds C++ object compilation. |
compiler.mk |
Configures gcc-only C++ builds. |
benches/nfs3.c |
Adapts kernels to the harness. |
aminet/benchwork.readme.in |
Updates package documentation. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
186
to
188
| complete = [all(isinstance(r[1][i], float) or r[1][i] == "not built" | ||
| for r in rows) | ||
| for i in range(len(columns))] |
A wrong output there no longer hides the geomean either, as Copilot noted on the PR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.

Adds two floating-point benchmarks from arczi84's 68k port of Need for Speed III, contributed under 0BSD:
projectObjectVertex(), a vertex through a 3x3 matrix, a translation and the perspective divide, every result checked by testing its float bits.PolygonClipWork::run(), a triangle clipped against the near plane and the four screen edges, with float compares, divides and interpolation.The kernels are C++, so only gcc builds them. Built with
-fno-exceptions -fno-rtti -fno-threadsafe-statics, they need nothing from libstdc++, and the existing C link takes them. vbcc and SAS/C skip both rows.kernel.cppis arczi84's file with hismain()replaced by two entry points. Those set the FPCR to single precision, as the game does, and restore it afterwards.source.sha256records the hash of the file as received.Results, 25 MHz 68040 under volamos:
-O2; gcc 16.2-rc14 beats 6.5.0b by 8.90% and 5.40%.-Osmakes both kernels 2.5 times slower: it keeps theGuardedFloatMathoperations out of line, so every float operation becomes a call.449f5330and4352ff86, the sums his README lists for x86-64 rather than his PiStorm ones. Every result is rounded to IEEE single, as SSE does, so the PiStorm (Emu68) may not apply the FPCR's precision control like a real FPU.Also in this PR:
--geomean-skipin the results tools), so it still covers the same 16 benchmarks for every compiler.Generated with Claude Code
Note
Low Risk
Benchmark and build/doc additions; no changes to existing benchmark logic beyond optional registration and reporting layout.
Overview
Adds two gcc-only C++ benchmarks from arczi84’s NFS3 68k port:
nfs3-project(vertex projection with guarded float math) andnfs3-clip(triangle clipping). Vendored code lives underthird_party/nfs3/with 0BSD license; the build wires inNFS3_SRCS, a%.cppcompile rule, andBENCH_HAVE_NFS3so vbcc/SAS/C builds skip these rows.The harness widens benchmark names to 16 columns, checks Ctrl-C between benchmarks (for newlib), and registers the new benches when NFS3 is enabled. Results tooling gains
--geomean-skipso compiler-comparison geomeans exclude benchmarks not every toolchain builds; README, RESULTS.md, charts, and Aminet readme are updated with the new rows and refreshed timings.Reviewed by Cursor Bugbot for commit 11a7984. Bugbot is set up for automated code reviews on this repo. Configure here.