Repository navigation
fix(shader): decide each resource branch condition once per plan - #11506
Conversation
MayDecide recursed into both operands of every LogicalAnd and LogicalOr without keeping what it had decided, and ran a full MayEvaluate at each step. A condition whose operands share their own operands, as when each level's two conjunctions read the level before, took time exponential in its depth. MayDecide now keeps each value's answer, and ExtractPlan keeps one record for all of a plan's blocks. Skyrim (PPSA03747) prepares shader 0x122151a400 when it calls sceAgcCreateInterpolantMapping on its New Game loading screen. Since 1c9a522 that preparation did not finish: the title's main thread spun in MayDecide, 20 to 29 frames deep, for the rest of a 300 s run, and no frame was presented after about 150 s. agc_shader_replay --prepare of the captured request runs past 60 s on main and takes 18 ms with this change.
github-actions
Bot
added
the
unblocks title
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
The focused regression evidence is useful: the new shared-condition test times out with the old evaluator and passes with this change, and the full selected suite reports the same RADV tessellation failure as main. The branch is based on current main (88795ed448), and local conventions plus git show --check pass.
Before approval, please add the paired AGC/shader-recompiler validation required by CONTRIBUTING.md: the same focused ctest -V run on current main (88795ed44801fa719eb2df990cbe1f11a1f9bd81) and this head (0dc6d3829544ec5b4933d59f3d2ef9819a703085), on the same machine/GPU, with VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, ANYPS5_NO_SHADER_CACHE=1 and ANYPS5_REQUIRE_VULKAN=1. Please include the GPU vendor/model, driver and OS, confirm neither log reports the requested-layer-missing message, and compare new VUID- lines. The current PR description reports tools/select_tests.py results but not this exact paired validation, and hosted product/relinker checks are skipped.
I found no implementation defect in the memoization path; changes requested for the missing required validation record.
|
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 Thanks. I added the paired run to Tested: the same 15 focused tests on I also added the title run on this base: Skyrim stops at 161 s on the cube refusal that #11507 handles. Could you take another look? |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
The requested paired validation is now documented with the same RX 9070 XT/RADV/Mesa setup on 88795ed and this head. Both focused runs pass (15/15); the loader confirms the validation layer was inserted, and neither log reports a missing layer or any lines. The reported suite failure is on the broader selected run only and is the same RADV tessellation segfault reproduced on . Applicable hosted checks (conventions, labels, eligibility, intake and report) pass; product/relinker jobs are skipped. I found no remaining blocking issue in the reviewed memoization change.
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
The requested paired validation is now documented with the same RX 9070 XT/RADV/Mesa setup on main 88795ed and this head. Both focused runs pass (15/15); the loader confirms the validation layer was inserted, and neither log reports a missing layer or any VUID- lines. The reported suite failure is on the broader selected run only and is the same RADV tessellation segfault reproduced on main. Applicable hosted checks (conventions, labels, eligibility, intake and report) pass; product/relinker jobs are skipped. I found no remaining blocking issue in the reviewed memoization change.
|
No-title Windows/NVIDIA test report for head Environment: Windows 11 Home 26200, 13th Gen Intel Core i5-13500H, NVIDIA GeForce RTX 4060 Laptop GPU, NVIDIA driver 596.21, WinLibs GCC 15.2.0 r7 (MinGW-w64 UCRT), Release. Official Khronos validation layer 1.4.363.0. Ran the same 15 tests listed in the PR description on both commits with
Both logs contain AI-assisted: yes (Kimi K3.1 Pro) |
✅ NVIDIA GPU test: passedPR head
Unstable on main (2, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (4, not counted against this PR)Passed on retry (1, failed in the parallel run, passed when re-run with a 360 s timeout; not counted against this PR)Validation IDs already present on main (5 IDs in 476 tests)Skipped (44)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. |
|
An update on the title result, on current AI-assisted: yes (Claude Code) |
|
Fresh Linux/RADV focused validation of Control
Exact source/dependency pins, integration patch, builds, verbose logs and JUnit, SHA-256 A plain-main Ghost of Yōtei startup host-thread sample also reached recursive |
github-actions
Bot
added
the
review queue
github-actions
Bot
removed
the
review queue
What
MayDecide(#8518) checks whether a resource block's branch condition can be decided at capture time. ForLogicalAndandLogicalOrit recurses into both operands, and every step starts with a fullMayEvaluate. Nothing is kept between steps. A condition whose operands share their own operands is therefore walked once per path, and each level that reads the level before it twice doubles the work.This PR keeps each value's answer:
MayDecidetakes a record of what it has already decided, andExtractPlanpasses one record for all of a plan's blocks. An answer doesn't depend on the block (MayDecidecallsMayEvaluatewithout an active mask), so sharing the record doesn't change any result.@JJenkx, this is the walk from #8518. Its answers are the same; they're just kept now.
Tested
main522e32e, 3 runs: the menus run at 60 fps. On the New Game loading screen, frames stop between 140 and 151 s, and the screen stays black until the timeout. Nothing is printed, andAPS5_STALL_WATCHDOG_SECS=15reports no stalled queue.sceAgcCreateInterpolantMapping→AgcDriverResolveShaderAbi→PrepareShader→ResourceMaterializer::ExtractPlan→MayDecide, 20 to 29 frames deep, underMayEvaluate.PrepareShaderWithDiagnostics(not in this PR) counted 3,818 preparations. Only one didn't finish: shader 0x122151a400, whose request I saved.main5ed5427 with this PR: the title gets past that point and stops at 155 s onguest cube texture descriptor holds fewer than 6 array slices (format 71, 512x512x5, ...). That's where it stopped onmain65ac1d3, before fix(shader): decide resource branch conditions from their uniform operand #8518 (see fix(libSceNgs2.native): accept a reverb voice with different input and output channel counts #10347).main5bdda36 with this PR: the same, at 156 s. With the cube descriptor fix in fix(agc): sample cube descriptors with fewer than six slices as six faces #11507 as well, it plays its opening, the cart ride to Helgen, until the 420 s limit of the run.main88795ed, this branch's base, with this PR: the same, at 161 s; with fix(agc): sample cube descriptors with fewer than six slices as six faces #11507 as well, the opening until the 420 s limit.agc_shader_replay --prepareof the saved request,ANYPS5_NO_SHADER_CACHE=1: onmainit was still running when I stopped it at 60 s. With this PR it takes 18 ms (3 runs).agc_shader_memory, newverifyMayDecideSharedConditions: 64 levels, where each level'sLogicalOrreads twoLogicalAnds of the level before and a lane comparison. With only lane comparisons,MayDecideanswers false. With a uniform conjunct at the bottom, it answers true. A shared record gives the same answers.main's evaluator, the test times out at 60 s. With this PR it passes in 0.16 s.main88795ed and this head 0dc6d38, same machine and GPU: Linux (Arch, kernel 7.2.11), AMD Radeon RX 9070 XT (gfx1201) with RADV, Mesa 26.2.5, GCC 16.2.1, Release.ctest -VwithVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation(Khronos validation layer 1.4.363; the loader logsInsert instance layer "VK_LAYER_KHRONOS_validation"),ANYPS5_NO_SHADER_CACHE=1andANYPS5_REQUIRE_VULKAN=1, on the tests around resource plans and their branch conditions:agc_shader_memory,agc_shader_disk_cache,agc_driver_graphics,agc_driver_branch_past_endpgm,agc_driver_image_table,agc_driver_image_table_cache,agc_driver_image_table_cache_bounds,agc_driver_image_table_reuse,agc_driver_image_table_reuse_draw_pipeline,agc_driver_image_table_vouched_lookups,agc_driver_image_table_vouch_queued_draws,agc_driver_compute_table_calls,agc_driver_compute_fs_table_calls,agc_driver_vertex_table_read_orderandagc_driver_vopc_compare_table.main: 15 of 15 pass. This head: 15 of 15 pass.was not found but was requested by env var VK_INSTANCE_LAYERS, and neither has aVUID-line.tools/select_tests.pyonmain88795ed,ANYPS5_REQUIRE_VULKAN=1: 1,500 of the 1,501 selected tests pass.agc_driver_tessellationsegfaults inside RADV, as it does onmainhere (agc_driver_tessellation segfaults on RADV with Mesa 26.2+: nir_opt_varyings corrupts the TCS input offset's use list (Mesa bug) #5444, Mesa 26.2.5).tools/check_conventions.pypasses.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)