Repository navigation
fix(shader): reuse folded bitcasts and NaN tests within blocks - #11564
Conversation
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
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.
|
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed exact head 09c36584fc458d0d1dc60f27f155947e06cfb27e; no actionable findings. The reuse maps are cleared at function and label boundaries, key casts by result type and source ID, key NaN tests by opcode/type/operand, and avoid reusing decorated results. The added SPIR-V module cases cover same/different values, types, sibling blocks and direct/group decorations. The detailed paired GPU validation report is scoped correctly and makes no title-performance claim; hosted eligibility, conventions, intake, labels and report checks pass, while the build/relinker/product jobs are skipped by path policy. I did not rerun the Vulkan tests locally.
|
Real-title follow-up on current main Matched current-base/change shader tests pass7/7 per arm, real Khronos insertion7+7, no VUIDs/skips. The CPU module regression test and four additional candidate graphics checks pass. Private base The same-machine/same-scene native pair uses identical saves/controller/renderer settings and complete200:30traces, normal recipe cache, no diagnostic profiler or debugger:
No demonstrated meaningful FPS improvement from this one pair; approximately14.8FPS at3840x2160 in both. Both clean revision/no dropped trace events, readable matching Welcome prompt over house, normal close240s/exit0, all saves/config/kernel64 restored and no remaining process. Native validation enabled, zero VUIDs. Tutorial gameplay/audio remain unverified. Paired official traces, summary lines, GPU test reports and aggregate shader statistics (32.6MB), SHA256 |
jotitadev
left a comment
There was a problem hiding this comment.
Ran this with ANYPS5_ENABLE_SPIRV_TOOLS=ON, the configuration the description leaves out and the CI build skipped. Linux, i7-8700, Ubuntu 26.04, GCC 15.2, Release. Head 09c36584f merged with --no-ff onto main 8df2c3ed0. lavapipe (Mesa 26.0.3).
- Red:
main+ only theSpirvModule.cppchanges failsagc_spirv_module3/3 with "5 check(s) failed", as in the description. - Green: with the PR,
agc_spirv_module,agc_spirv_optimizer(+_full,_none),agc_redundant_value_eliminationandagc_spirv13_imagepass 3/3.agc_spirv_optimizeralso checks the bitcast, copy and constant counts after specialization, and they are unchanged. - Suite:
^agcplusspirv_validator_exitwith SPIRV-Tools ON (1053 tests, display/flip/lds_atomics64 excluded) gives 1051/1053 both onmainand with this PR. The two failures (agc_driver_multisample_retirement,agc_driver_guest_memory_lifetime_converted) are the same onmain.
GPUs tested: lavapipe (Mesa 26.0.3).
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
Title data on RADV, for the GPU-time side of this change. The result is neutral. Builds. Dusk Diver 2 (PPSA06565) title menu, RX 6650 XT, RADV Mesa 26.2.4, Linux 7.2.9.
Each arm had its own warmed shader cache, and the runs alternated, 90 s each, with
There is no consistent difference. The spread between runs of the same build (about ±5 %) is larger than any effect. This title's heaviest shaders are dominated by the NaN tests this PR reuses: This says nothing about the PR's purpose of fixing #4287's optimizer-off failure, nor about other drivers. AI-assisted: yes (Claude Code). |
What
Late SPIR-V specialization can fold different operands to the same source after IR common-subexpression elimination. This leaves duplicate bitcasts and NaN predicates in the emitted block. Reuse identical casts and NaN tests within each basic block; keep different input/result types, sibling blocks and directly/group-decorated results separate.
This independently fixes the late-fold redundancy behind the existing
agc_redundant_value_eliminationfailure seen with #4287 andANYPS5_ENABLE_SPIRV_TOOLS=OFF. It does not incorporate or depend on #4287. No title FPS improvement is claimed.Overlap tool found no confirmed overlaps. Its uncompared #8004 changes shared opcode/result/operand helpers, not this folding logic; reviewed manually. #7677 handles round-trip casts in a different pass.
Tested
Independent SPIRV-Tools-enabled validation by jotitadev: exact head
09c36584fmerged onto main8df2c3ed0, Ubuntu 26.04 / GCC 15.2 Release,ANYPS5_ENABLE_SPIRV_TOOLS=ON, lavapipe Mesa 26.0.3. The test-only red arm failsagc_spirv_modulein all 3 runs; the full PR passes the module, optimizer (default/full/none), redundant-value-elimination and SPIR-V 1.3 image checks in all 3 runs. Optimizer bitcast/copy/constant counts remain unchanged after specialization. The 1,053-test suite gives 1,051/1,053 on both main and the PR: the two failures are unchanged baseline failures (agc_driver_multisample_retirement,agc_driver_guest_memory_lifetime_converted). These are reviewer-run results and identify their exact tested source; they are not a title-performance claim.Linux Release, GCC 16.2.1; base
e0b13f2990f31a949a6da21d81a7789ebdfeaa26, change09c36584fc458d0d1dc60f27f155947e06cfb27e. Optional SPIRV-Tools optimizer disabled throughout these focused tests.agc_spirv_moduleandagc_redundant_value_eliminationpass on the change.ANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1,VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation; Mesa shader cache also disabled.spirv-val --target-env vulkan1.3, covering block dominance, types and decorations.Evidence: source pins, paired verbose logs, regression outputs, generated toy modules and reproduction scripts. SHA256:
1df80e10d1252a976cecf493d3c0168610bf2e7094cf30e29bc0ada78f7bad15. Archive publicly downloaded and hash-verified; no game assets or game shader captures included.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)