Repository navigation
perf(shader): simplify accuracy-series distance checks - #13855
Conversation
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed — looks good
The biased unsigned comparison preserves the original four-bit-pattern threshold, and the paired capture calculations and all eight published trace digests reproduce.
What I ran
Reviewed head 2237678f369c8a67a605ab15e3fb74e4ef6cc52a against PR base c3c45d73abd34dca81da6bad87cb6d36ea437a66; the current-main merge preview was clean. The measured pair is based on c5e389a12647ae98b21b55dc522ab4d6d3d0943b and candidate 2237678f369c8a67a605ab15e3fb74e4ef6cc52a; the PR explicitly limits the timing result to that pair. Environment: Linux x86-64, Python 3.11.2; no compiler or build was run.
python3 tools/pr_precheck.py --repo boykopovar/AnyPS5 --pr 13855
python3 tools/pr_stack_check.py 13855 --repo boykopovar/AnyPS5 --output-dir /tmp/anyps5-pr-13855-stack
python3 tools/trace_digest.py /tmp/anyps5-13855-trace-audit/aps5-trace-c5e389a12647-244393.json.gz --summary /tmp/anyps5-13855-trace-audit/A1-aps5-summary.jsonl
python3 tools/performance_compare.py /tmp/anyps5-13855-trace-audit/A1-aps5-summary.jsonl /tmp/anyps5-13855-trace-audit/B1-aps5-summary.jsonl --baseline-trace aps5-trace-c5e389a12647-244393.json.gz --candidate-trace aps5-trace-2237678f369c-245198.json.gz --jsonThe trace-digest and comparison commands were repeated for all eight captures and four pairs, respectively.
Results
| Check | Result |
|---|---|
| Diff and conventions | Passed |
| Current-main merge preview | Clean; the branch's measured base is older, and the report does not claim the timing transfers to current main |
| Trace digest | All 8 regenerated outputs exactly matched the linked digest files |
| Paired performance comparison | All four reductions exactly matched the linked comparison record |
| Strict context comparison | All four pairs were rejected because window.first_present differs (A/B: 669/657, 625/673, 639/633, 649/668); the other recorded machine, title, resolution, and fixed seconds 30:12 window fields match |
| Hosted CI | Linux and Windows jobs skipped; platform-specific jobs were queued at refresh |
| Local build/tests | Not run |
Unedited trace and comparison audit output
A1: summary_commit=c5e389a12647 summary_trace_match=True posted_digest_exact=True
A2: summary_commit=c5e389a12647 summary_trace_match=True posted_digest_exact=True
A3: summary_commit=c5e389a12647 summary_trace_match=True posted_digest_exact=True
A4: summary_commit=c5e389a12647 summary_trace_match=True posted_digest_exact=True
B1: summary_commit=2237678f369c summary_trace_match=True posted_digest_exact=True
B2: summary_commit=2237678f369c summary_trace_match=True posted_digest_exact=True
B3: summary_commit=2237678f369c summary_trace_match=True posted_digest_exact=True
B4: summary_commit=2237678f369c summary_trace_match=True posted_digest_exact=True
A1/B1: paired reduction 0.201362769448% matches posted 0.201362769448%: True
A2/B2: paired reduction 0.259748214819% matches posted 0.259748214819%: True
A3/B3: paired reduction 0.174336986809% matches posted 0.174336986809%: True
A4/B4: paired reduction 0.076679991746% matches posted 0.076679991746%: True
performance_compare.py --require-matching-context: recorded configuration differs: window.first_present (all four pairs)
Sources
The changed selector logic is in SpirvAluEmitterMath.cpp. The immutable paired comparison record and measurement notes identify the exact measured revisions and limits.
Not verified
I did not run the reported Vulkan CTest fixtures or differential helper locally, build the project, verify the physical GPU/scene, or test on another GPU or console. The author reports those tests on the measured baseline/candidate pair; the timing result is limited to that pair and setup.
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
Windows GPU run of head This PR changes no test files, so I picked the existing GPU execution tests that run the code it changes:
AI-assisted: yes (Claude Code). |
|
NVIDIA and Intel on Windows, for head Host: Windows 11 Pro 26200, i7-9750H, WinLibs GCC 15.2.0 (
Only these test targets were built, not the full suite, and no title was run. AI-assisted. |
✅ NVIDIA GPU test: passedPR head
Failing on main too (2, not counted against this PR)Validation IDs already present on main (3 IDs in 387 tests)Skipped (48)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
|
@adriadam10 @oneandonlydean These five independent PRs have approvals on their current heads and no failing latest hosted checks: #13276 ( I checked the combined Git merge preview against main Could a collaborator run the selected full-platform CI batch and merge the individual PRs if that validation and review are satisfactory? My account has no upstream write or workflow-dispatch permission. gh workflow run build.yml --repo boykopovar/AnyPS5 --ref main --field pr='13276 13821 13826 13841 13855' --field select=true#11952 is excluded while its performance-review request remains open. #12299 is excluded while its #8716 prerequisite and new CI run are pending. #13268 and #13517 still need their own review decisions. AI-assisted: yes (Codex). |
What
The trig and log accuracy selectors calculate both unsigned differences and their minimum only to decide whether the native result is more than four bit-pattern units from the exact series. Replace that calculation with one biased unsigned interval check.
(valueBits - seriesBits) + 4and select the series when it exceeds 8.Let
d = a-bmodulo 2^32.min(d,-d) <= 4iffdis in[0,4]or[2^32-4,2^32-1], exactly iff(d+4) mod 2^32 <= 8. Complementing gives the unchanged far predicate for every pair of bit patterns, including wraparound, signed zero, infinities and NaNs. Eight is the biased interval width, not a relaxed error tolerance. SPIR-V OpIAdd/OpISub define low-order N-bit results. This preserves the corrections introduced by #3770 and #2880 and the near-one guard from #10336; no new arithmetic approximation or lane/memory behavior is introduced.Tested
Main
c5e389a12647versus candidate2237678f369c, God of War Sons of Sparta PPSA28997 v01.008.001, saved camp after Continue, 1920×1080 guest output, Wayland/mailbox. AMD Ryzen 5 7640HS / Radeon 760M integrated GPU with shared system memory, RADV Mesa 25.0.7-2+deb13u1, Debian 13.7 / Linux 7.2.12-x64v3-xanmod1.Mean frame time 45.719473 → 45.638076 ms. Four paired percentage reductions average 0.178032%, two-sided 95% Student-t CI [0.056473%, 0.299591%], df=3. Mean run FPS separately: 21.872518 → 21.911536.
Within-arm ranges: main 0.008939 ms, candidate 0.074771 ms; sample SDs 0.004167 and 0.031384 ms. Fixed primary metric: window duration / presents per run. Fixed order warm A, warm B, A1, B1, B2, A2, B3, A3, A4, B4; all ten attempts valid, no exclusions, replacements or appended runs. Acceptance required positive paired mean and lower confidence bound, with no minimum percentage. Frames are not independent repetitions. This is a small measured improvement on one setup/scene with four pairs; no universal or combined-patch gain is claimed.
All eight traces, unchanged summaries, complete trace_digest reports, protocol, provenance hashes and measurement-hook source. Same save, scene/input route, resolution, focus/window geometry and separate warmed per-arm cache seeds; unchanged manifests through every window. 51 native libraries and nine unique guest modules, audited as full-path lists with duplicate detection and namespace hashes, plus actual loaded RADV ICD. The native count increased from historical50 because upstream Audio3d now links
libkernel_sys; the complete dependency closure is checked and identical between arms. No drops or validation/profiling during timing; stdout remained a pseudo-terminal. Identical readiness-only hooks operate outside timed windows and are excluded from production; VideoOut differs only in revision text. Tracedirty:nullreflects dependency-symlink status failure; exact source/patch/binary provenance is retained.Matched main/candidate/final hook-free
SinCosReduction,SinCosNearZero,Vop1FloatUnaryandLogNearOnefixtures pass with verbose CTest, required Vulkan, shader cache disabled and Khronos instance/device layers. Four Radeon selections per arm, zero skips, missing-layer warnings or VUIDs. The retained trig differential helper with fresh libraries gives 29,184 bit-identical outputs across supported FP/fused settings and unchanged errors for the two unsupported denormal modes. Existing committed fixtures cover f32/f16, near-one boundaries, cardinal values, signed zero, subnormals and NaNs/infinities; no duplicate framework is added.Actual warmed production modules compiled with the same retained fragment-GPL inspector change 6,483 → 6,433 and 6,501 → 6,461 instructions, both 48 → 40 VGPRs, with unchanged 128 SGPRs, 56 branches and zero spills. This is compiler-only evidence from serialized game modules, not loaded LTO-code or occupancy measurement. A private exact-transform falsifier also shrinks a log module 3,966 → 3,952 instructions at unchanged allocation. The elapsed cohort above independently establishes the scene-level result; no game-derived shader or ISA is published. Other GPUs were not tested locally, and historical full-game vertex/index-binding VUIDs remain unrelated; no clean full-game validation claim.
Conventions pass. Full overlap scan covers 1,621 open PRs and 11 shared-file branches. Stale #5021 and #12324 require manual review; their shared patches are identical to the reviewed lane/NaN and wave-scan changes, with no distance-predicate overlap. #13268, #13826 and #13841 change independent cosine/cardinal/sine mechanisms; no required dependency. Upstream advanced to
c3c45d73abd3during final review, including unrelated AGC draw-relocation/runtime changes. This PR retains its exact measured base/head; timing is not transferred to that later runtime baseline. The full scan's merge preview is clean.Main/candidate summary lines and trace_digest excerpts (all runs in the bundle)
{"commit":"c5e389a12647","dirty":null,"env":{"APS5_NO_PAD_INPUT":"1","APS5_PAD_INPUT_FILE":"<path>","APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"30:12"},"gpu":"AMD Radeon 760M (RADV PHOENIX)","driver":"radv Mesa 25.0.7-2+deb13u1","os":"Linux 7.2.12-x64v3-xanmod1","cpu":"AMD Ryzen 5 7640HS w/ Radeon 760M Graphics","cpu_threads":12,"memory_mib":23820,"title_id":"PPSA28997","title":"God of War Sons of Sparta","content_version":"01.008.001","resolution":"1920x1080","window":{"spec":"seconds 30:12","first_present":669,"ms":12024.162},"presents":263,"frame_ms":{"p50":45.672,"p90":46.222,"p99":46.803,"max":51.92},"gpu_busy_ms_per_present":42.543,"worker_busy_pct":{"queue worker 0x0":78.331},"top_phases_ms_per_frame":{"Driver.Packet":35.683,"Driver.Packet.draw":33.211,"Driver.Draw":33.174,"Driver.Draw.draw_enqueued":29.579,"Recorder.Sync":28.565,"VideoOut.Flip":9.023,"Driver.Present":9.013,"VideoOut.Flip.present":8.986},"trace_dropped":false,"trace":"aps5-trace-c5e389a12647-244393.json.gz"} {"commit":"2237678f369c","dirty":null,"env":{"APS5_NO_PAD_INPUT":"1","APS5_PAD_INPUT_FILE":"<path>","APS5_TRACE_DIR":"<path>","APS5_TRACE_SECONDS":"30:12"},"gpu":"AMD Radeon 760M (RADV PHOENIX)","driver":"radv Mesa 25.0.7-2+deb13u1","os":"Linux 7.2.12-x64v3-xanmod1","cpu":"AMD Ryzen 5 7640HS w/ Radeon 760M Graphics","cpu_threads":12,"memory_mib":23820,"title_id":"PPSA28997","title":"God of War Sons of Sparta","content_version":"01.008.001","resolution":"1920x1080","window":{"spec":"seconds 30:12","first_present":657,"ms":12045.577},"presents":264,"frame_ms":{"p50":45.553,"p90":46.168,"p99":47.429,"max":48.194},"gpu_busy_ms_per_present":42.494,"worker_busy_pct":{"queue worker 0x0":78.31},"top_phases_ms_per_frame":{"Driver.Packet":35.603,"Driver.Packet.draw":33.094,"Driver.Draw":33.054,"Driver.Draw.draw_enqueued":29.297,"Recorder.Sync":28.279,"VideoOut.Flip":9.026,"Driver.Present":9.017,"VideoOut.Flip.present":8.99},"trace_dropped":false,"trace":"aps5-trace-2237678f369c-245198.json.gz"}seconds 30:12 from present 669: 263 presents, frame p50 45.7 / p90 46.2 / p99 46.8 / max 51.9 ms, GPU busy 42.5 ms per present
AMD Radeon 760M (RADV PHOENIX), radv Mesa 25.0.7-2+deb13u1, Linux 7.2.12-x64v3-xanmod1, 1920x1080, commit c5e389a, dropped: no
seconds 30:12 from present 657: 264 presents, frame p50 45.6 / p90 46.2 / p99 47.4 / max 48.2 ms, GPU busy 42.5 ms per present
AMD Radeon 760M (RADV PHOENIX), radv Mesa 25.0.7-2+deb13u1, Linux 7.2.12-x64v3-xanmod1, 1920x1080, commit 2237678, dropped: no
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)