Repository navigation
Conversation
|
Which shader replaces a value with thousands of uses? The table times |
|
Acknowledge that your measurements are synthetic and that real shader replay timings remain unavailable. |
adriadam10
left a comment
There was a problem hiding this comment.
Checked on the PR merged onto current main: full build and ctest are green apart from agc_driver_tessellation (lavapipe SEGFAULT, unrelated to this code), shader_ir_use_rewrite passes, and it fails with IrValue.cpp reverted. The rewrite is equivalent to the old loop (use order, nullptr, self-replacement, phi/identity).
One thing is missing before this can merge:
- The benefit is only shown on synthetic graphs (
ReplaceAllUsesWithalone, 4,000–128,000 references). As @oneandonlydean asked, perf PRs need before/after numbers on a real case (CONTRIBUTING, perf PRs): the recompile time of a dumped request throughagc_shader_replayon main vs this branch (e.g. the Stray requests from #3504/#4331, or any title with a high-fan-out value), plus a profile showingReplaceAllUsesWithis a meaningful share of it. Without it we can't tell this is not a no-op in practice (#3726 touched the same helper and was closed without that).
Happy to merge once that number is in the PR.
…time CPU (#482) * perf(shader): rewrite high-fan-out IR uses in linear time Ported from upstream PR boykopovar#4035 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(shader): index specialization tables by SPIR-V id Ported from upstream PR boykopovar#4772 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(shader): honor disabled optimization during specialization Ported from upstream PR boykopovar#4827 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(shader): discard helper functions unreachable after specialization Ported from upstream PR boykopovar#5064 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(shader): drop module constants nothing references after specialization Ported from upstream PR boykopovar#5315 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(shader): cap SPIR-V scalar replacement Ported from upstream PR boykopovar#4288 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(recompiler): iterate structurizer fixpoints in post-order and bound shared merge splitting growth Includes the stacked upstream commit that structurizes refused control flow with reach flags, which this change builds on. Ported from upstream PR boykopovar#5028 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(shader): emit image_bvh_intersect_ray as one shared function per variant Ported from upstream PR boykopovar#5455 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(shader): avoid redundancy traversal map copies Patches the bundled SPIRV-Tools redundancy elimination (only built with ANYPS5_ENABLE_SPIRV_TOOLS=ON). Ported from upstream PR boykopovar#5511 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(metal): accept a specialized BDA module that dropped one of its two BDA descriptors Specialization removes descriptor variables the final module no longer references. With helper functions now discarded after specialization, a compute module can drop its page-table variable while keeping the fault buffer, and the bridge then rejected the dispatch ("BDA ABI requires page-table and fault-buffer descriptor metadata"). Require at least one of the two roles instead of both; the module's own descriptors are still checked against the prepared plan. Not a port; needed by the port of upstream PR boykopovar#5064. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
Closing: no title is known to reach this. With the queue at 700+ open PRs, please hold off on new PRs that don't name a title needing the change until it's down. If a title stops on this on current main, reopen with the title ID and the log line. |
|
Evidence follow-up: the existing timings are synthetic IR-helper measurements. I still do not have a real title request with before/after I checked the cited Stray reports: #3504 was closed as fixed by #4331, which changes To complete the requested comparison, could a contributor attach a downloadable AI-assisted: yes (Codex). |
What
The maintainer identified shader recompilation overhead as a priority (comment). This change removes an unclaimed quadratic operation in the shared IR rewrite primitive.
IrValue::ReplaceAllUsesWithpreviously calledReplaceArgumentonce per operand. Each call searched and erased entries in the source's use vectors; replacing a highly shared value therefore cost O(n²). Update the referring operands in one reverse traversal, append the replacement's uses in the same order, and clear the old lists once. Existing operands, use ordering, opcodes, metadata, phi predecessors and parent ownership are preserved.Reserve both destination lists before changing references, with geometric capacity growth. Allocation failure leaves the graph intact; the old implementation can remove a source use and redirect its argument before a destination allocation fails.
No instruction lowering, shader-cache policy or optimization proof is changed. #3726 changes an SRT caller to use this existing helper; it edits different files and is not a dependency. The reported masked-walk, host-import-budget and Windows red-zone changes already have other proposals; this PR does not duplicate them.
Tested
Linux x86-64, GCC 13.3, C++20, AMD EPYC 9V74 host.
-O2 -Wall -Wextra -Werror, including duplicate operands and existing replacement uses, self/null replacements, phi/identity users, follow-up edits, both destination allocation failures, geometric growth, and 200,000 references.IrValue.cppfails the new allocation-failure-integrity regression; the fixed implementation passes.shader_ir_use_rewriteunderBUILD_TESTING, including relinker-only builds, with a 20-second timeout. The complete release suite runs in approximately 0.20 s locally.Five alternating baseline/fixed runs per benchmark size, using fresh graphs and excluding construction/destruction:
Every run verifies all redirected operands and final use counts. Separate tests of 2,000–32,000 small two-reference rewrites into one replacement improved by 1.23–1.45x, checking that repeated reservations do not introduce quadratic growth.
These timings isolate the IR helper. End-to-end shader compilation, byte-identical SPIR-V on title captures, retail-title FPS, full runtime CTest and Windows/macOS execution remain unverified locally. No game-wide speedup or direct resolution of #3504/#2809/#361 is claimed.
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository (attach them to this PR)