Repository navigation
fix(shader): prove masked lanes unread for every select in one backward pass - #4331
Conversation
github-actions
Bot
added
needs rebase
|
@oneandonlydean measured on this PR's head
The two
I haven't run the title with it yet. That needs a rebuilt set, and I'll add it here if it shows anything. AI-assisted: yes. |
…rd pass MaskedSelectEliminator walked forward from each select to see whether the lanes its mask excludes are read. Since fda4aa9 the walks pass explicit-LOD gathers, size queries and guarded image writes, so in a large pixel shader most of them cover the rest of the program, and the cost grows with the square of its size (boykopovar#3504). The pass now computes, for every value, the set of masks under which it is unread, from its uses backwards: a use guarded by a condition contributes the masks that condition implies, a lane-local use adds what its result is unread under, and any other use contributes nothing. A select is dropped when its mask is in its own set. Loops are solved as strongly connected components. The rules for guards, lane-local uses, loops, whole quad masks, selected execs, rebuilt mask words and guarded first-lane reads are the ones the walk applied. A first-lane read passes the masks its read mask implies when its own result is unread outside that read mask, which is solved once per read in the same way. The walk's step limit, its 64-level implication depth and its first-lane nesting depth are gone, so a proof no longer depends on the order of the questions.
dbad071 to
d37463e
Compare
|
@ThatsLeo thanks, that is the number this needed. The branch is rebased onto |
|
@oneandonlydean on the rebased head
AI-assisted: yes. |
|
Independent soundness check of this branch with the masked-select fuzzer ( Setup: this PR's head Soundness, seeds 10,000,000 to 10,499,999, both wave sizes, 1,000,000 programs per tree:
No mismatch and no exception. The same fuzzer has run more than 19 M programs on Same programs through both passes (240,000 programs, seeds 10,000,000 to 10,119,999): the IR after the pass is identical in all of them, and the instruction count is identical (26,390,091 on both sides). The removed counts differ in 1,325 programs (770 higher on this branch, 555 lower), with identical IR in every one of those, so that is only a difference in how the stat is counted. Timing, CPU time on the same machine:
Not covered: the fuzz programs are too small to reach the old step and depth limits, so the extra selects dropped beyond them are not checked by this fuzzer. Your comparison against the walk with its limits lifted is the check for those. AI-assisted: yes (Claude Code) |
|
I replayed 148 requests from two more titles with and without this PR. The tool's output is identical, including every SPIR-V word count, and the replay is a little faster with it. Linux, GCC 16.2.1 Release,
Kena's one failure is the same on both: The 13 tests matching |
4 participants
What
Refs #3504. That regression came in with #3279, which I carried, so this is the fix for it. I do not have Stray's requests: @ThatsLeo, could you time
0x200f780000and0x200f7a0000on this branch, and check that0x2013010000and the rest of your 43 still compile?MaskedSelectEliminatorwalked forward from every select to see whether the lanes its mask excludes are read. Sincefda4aa90(bisected by @ThatsLeo) the walks pass explicit-LOD gathers, size queries and guarded image writes, so most of them cover the rest of the program. The cost is selects times program size.The pass now computes the answer for all selects at once. For every value it keeps the set of masks under which the value is unread, built from its uses backwards:
LogicalAnd, an access with a trailing exec) contributes the masks that condition implies;A select is dropped when its mask is in its own set. Values and conditions that depend on themselves through loop phis are solved per strongly connected component, starting from "every mask". Selects are settled users first, so a dropped select is already gone when the values it read are settled, as in the walk's reverse sweep.
The rules are the walk's: the same guarded uses, lane-local list, phi and loop conditions, ballot and mask-word forms, whole quad masks (#3391), and the selected execs, rebuilt mask words and guarded first-lane reads of #3739. A first-lane read passes the masks its read mask implies when its own result is unread outside that read mask; that is solved once per read, in the same way, with #3739's same-block rule for a loop-local read mask. Three things are gone: the step limit per walk, the 64-level depth of the implication check and the nesting depth of first-lane proofs. A select that the walk kept only because it ran into one of them is now dropped, which includes a first-lane read whose result only reaches itself around a loop. One case is refused instead of counted: a first-lane proof that needs a read mask of a kind (loop-local or not, whole-quad or not) that is already being solved. It is not a budget: no select is kept for lack of time.
Measured on a synthetic pixel shader of the reported shape (each unit compares, selects, gathers with LOD zero and writes a register under the exec; one image write at the end), Release, same machine:
main7a84b22The visit and implication counts on
maincome from counters added locally. Both trees drop the same 16,000 selects in the last row.Relation to #3550 and #4031: #3550 keeps the walk and stops proving when a per-shader budget is spent, so the selects it keeps depend on the multiple (1,602 of 9,232 dropped at 32x on
0x200f780000, and0x2013010000refused below 128x, per @ThatsLeo's tables in #3504). With this change there is no budget to pick, and #3550 should not be needed for #3504; if it goes in anyway it conflicts with this one. #4031 is independent: it is still needed for selects that no proof can drop (Palworld's0x1b40e5c000, Minecraft Dungeons II), but not to make a budget safe.Cost: the sets are explicit, so memory grows with values times the masks their guards imply. A chain of 8,000 nested kills takes 0.09 s and about 150 MB here (3.4 s on
main).Tested
Linux (CachyOS, GCC 16.2.1, Release,
BUILD_TESTING=ON,ANYPS5_ENABLE_SPIRV_TOOLS=ON), RTX 4090 (NVIDIA 610.57.04). The commit was rebased over #3739 and carries its three rules, so everything in the first list was run again.On
main8c4a295 plus this commit:ctest -R "agc|shader": 442 of 442 pass.agc_masked_selectpasses with every case from fix(shader): prove masked lanes unread across exec phis, loops and image operations #3279, fix(shader): drop masked selects proven unobserved through whole quad mode #3391 and fix(shader): prove masked writes through guarded first-lane reads #3739 unchanged. Three cases are new: an exec narrowed by 300 kills still drops the select, 300 kills rooted at a wider exec keep it, and a program of 4,000 units costs at most 9 times the steps of one of 500 units (it is 8 times). WithMaskedSelectEliminator.cppfrommain, the 300-kill case keeps the select (depth limit) and the step case fails; that file takes 4.4 s for the test onmainagainst 0.03 s here.tools/check_conventions.py --base origin/mainpasses.On
main436e310 with and without this commit (nothing undercore/shaderchanged between 436e310 and 8c4a295):agc_shader_replay --spv: every module is byte-identical tomain(30,722,012 words), in 277 s against 282 s. These are the ones of my 163 that compile on thismain; 7 more stop at the clip check of 9db923e on both trees. At least one of the 143 has first-lane reads.main's walk with its limits lifted and a first-lane read that only reaches itself around a loop taken as unread: the programs after the pass are identical in all of them. 10 deliberate errors in the three rules from fix(shader): prove masked writes through guarded first-lane reads #3739 (the wrong arm of a selected exec, mask words matched on the low word only, a first-lane result taken as always unread, and so on) are each caught by the same comparison within 40,000 programs. The harness is not part of the commit.mainand two with this commit (menu, new game, two rooms; 220 s): all 7 visual checkpoints pass on all three, 36.5, 33.8 and 39.1 fps in the menu, 37.7, 29.2 and 40.6 in gameplay (frames dumped on the CPU; other work was running on the machine during the slow one), no error line, no skipped draw, no Xid. Each run started with an empty shader cache and compiled 107 modules (3,251,533 words): the same 107 keys, every module byte-identical.Before the rebase, with the commit as it was without #3739's rules:
ctestonmaind0ca839 with and without it (configured as CI does): 921 pass and 4 skip (video_out_flip_*) of 925 on both, no failure.maina11c8f7 with fix(shader): drop masked selects proven unobserved through whole quad mode #3391, feat(agc): depth planes sampled as 2D arrays, cubes and integer views #425, fix(libSceAgcDriver): retire a depth surface once another writer covers its memory #454, feat(agc): host import windows, least-recent eviction under the budget #290, perf(agc): keep unchanged guest layers of a depth plane copy #2490, perf(agc): skip the depth plane copy when no slice changed #2491 and five local changes), with and without it, scripted to the title, the menu and the first level: 28.4 and 28.2 fps at the title, 27.8 and 26.3 in the menu, 26.5 and 25.4 in the level with a warm cache, the same frames by eye, the same checkpoint scores, no error line, no Xid. With an empty shader cache both compile 1,925 modules (280,313,141 words) with the same keys; 1,607 are byte-identical and the other 318 differ only in one 64-bit constant that holds a host address and differs the same way between two runs of one tree. The title scene starts 50 to 57 s after launch with it and 54 to 87 s without (two cold runs each, other work on the machine in three of them), 43 to 45 s on both with a warm cache; the first level loads in 26 to 30 s on both. No start-up difference shows on this title beyond the spread between runs.Not tested: Stray itself and its requests (see above), Windows, AMD and Intel; the full
ctestand the 3D title on the rebased commit.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)