Repository navigation
Add wipeout-tris bench routine - #1
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7098cfd3c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| The Wipeout rewrite publishes no license; its README asks that the code not | ||
| be used in a commercial release. |
There was a problem hiding this comment.
This commit adds kernel.c and several Wipeout headers to the distributed source tree, while the new vendoring README states that upstream publishes no license. A request not to use the code commercially is not a license to copy, modify, or redistribute it, so merging this leaves BenchWork distributing code without established permission; obtain and include the upstream terms or remove the vendored sources.
Useful? React with 👍 / 👎.
| static tris_t tris_buffer[RENDER_TRIS_BUFFER_CAPACITY] __attribute__((aligned(8))); | ||
| static uint32_t tris_len; | ||
| static render_texture_t textures[TEXTURES_MAX]; |
There was a problem hiding this comment.
When --fastmem is selected, only allocations routed through bench_alloc() are required to be MEMF_FAST; this staging buffer (and the adjacent texture table) instead live in the executable's BSS. On systems where that BSS is placed in chip or otherwise non-fast memory, every triangle assignment still writes this uncontrolled buffer, so the new benchmark can report memory-placement effects despite the option intended to prevent incomparable timings. Allocate this working storage through the harness or otherwise require fast memory for it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The 1024-triangle pattern never switches textures, and the vendored source lacks redistribution terms.
Review effort: Balanced
Findings: 1
Open (2)
What changed in this PR
Adds a Wipeout triangle-submission benchmark to measure UV scaling, color clamping, buffering, and structure-copy performance.
Changes:
- Vendors the extracted Wipeout rendering kernel and required headers.
- Adds benchmark inputs, validation, texture-switch patterns, and registration.
- Updates build configuration and benchmark documentation.
| File | Description |
|---|---|
Makefile |
Builds the new benchmark and kernel. |
README.md |
Documents wipeout-tris. |
benches/wipeout.c |
Implements benchmark setup, execution, and validation. |
src/bench.h |
Declares the benchmark. |
src/main.c |
Registers the benchmark. |
third_party/wipeout/README.md |
Documents provenance and licensing status. |
third_party/wipeout/bench.h |
Defines the kernel/harness interface. |
third_party/wipeout/kernel.c |
Provides the extracted rendering workload. |
third_party/wipeout/render_gl_legacy_types.h |
Defines triangle and vertex types. |
third_party/wipeout/source.sha256 |
Records the source-body hash. |
third_party/wipeout/types.h |
Provides supporting types and math helpers. |
third_party/wipeout/utils.h |
Provides supporting utility macros and functions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| The Wipeout rewrite publishes no license; its README asks that the code not | ||
| be used in a commercial release. |
| (i * 3 + j) % 256, i % 7 ? 255 : 0 }; | ||
| } | ||
| for (unsigned m = 0; m < 3; m++) | ||
| tex[m * N + i] = (i / RUNS[m]) % 8; |
codewiz
changed the title
ffedec4 to
5e2e6dc
Compare
render_push_tris() from arczi84's Wipeout port, as he packaged it for m68k-amigaos-gcc#89: float UV scaling, byte clamps and a 96-byte struct copy per triangle, under three texture-switch patterns. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
5e2e6dc to
e5a3df0
Compare
Adds
wipeout-tris:render_push_tris()from @arczi84's Wipeout port, as packaged for AmigaPorts/m68k-amigaos-gcc#89. The kernel and its headers are vendored verbatim underthird_party/wipeout/;benches/wipeout.cprovides the flush callback, runs the three texture-switch patterns (every 1024, 8 and 1 triangles) and checks one extra pass per pattern against a scalar reference.Under volamos (68040, 25 MHz, cycle counted), rc13 soft float takes 1164 ms vs 1112 ms for gcc 6.5, with identical check values across compilers and with
-mhard-float.@arczi84: the Wipeout rewrite publishes no license, so before merging I would like to add a LICENSE file under
third_party/wipeout/. Which terms should it carry for the extracted function and headers?