Repository navigation
perf(shader): intersect dominator sets as bitsets - #6025
Conversation
Structurizing recomputes dominators after every CFG rewrite, and each block started from a vector of every block id. The same fixed point now runs on bitsets and still writes the sorted id lists. Co-authored-by: Cursor <cursoragent@cursor.com>
CnRJay
left a comment
There was a problem hiding this comment.
This is a perf( PR with no measurement at all, and per the description no test was run either. Neither part is reviewable as a performance change yet.
- No numbers. There's no structurizer time or shader preparation time before and after, not even a micro-benchmark on a large real program. A bitset intersection is very likely faster than sorted-vector intersection for large CFGs. But the change also adds an
analysisVectorsconversion back to sorted id lists after every fixed point, and allocatescountbitsets ofcountbits per analysis, which is O(n²) memory. On small CFGs, which most shaders have, that can come out even or slower. Only a measurement on real shaders shows which way it goes. - Not tested.
agc_structurizerandagc_shader_control_flowcover code that, if wrong, produces silently wrong shaders. They have to run before a review is worth anyone's time. The CI workflow on your fork can run them if you don't have the toolchain locally. - Two implementations behind a switch. Keeping the old vector path under
APS5_NO_PERF_DOM_INTERSECTIONdoubles the code to maintain in a correctness-critical pass. Diagnostic switches make sense for behaviour that's uncertain on some drivers. A pure algorithmic replacement with identical output should replace the old code, with a test that compares the two on a set of real CFGs.
If dominator computation is actually hot here, the bigger win is the algorithm rather than the set representation. Cooper–Harvey–Kennedy's iterative immediate-dominator algorithm stores one idom per block, intersects by walking the tree, and needs O(n) memory. Dominance queries can then be answered from the tree. Recomputing only after the rewrites that change the CFG would also cut the number of runs.
jotitadev
left a comment
There was a problem hiding this comment.
A first run of the tests the description lists as not run, on Linux, plus a rough timing with the new path switched on and off in the same binary.
Machine: NVIDIA GeForce RTX 3070, driver 595.99.02, Vulkan 1.4.329, Ubuntu 26.04 (kernel 7.0.0-38), i7-8700. GCC 15.2, Release, BUILD_TESTING=ON. Head b44ee9c45 merged onto main 31cbb55d7, and again onto main 5d9dcc42d for the timing. No conflicts, no warnings. ANYPS5_GPU="NVIDIA GeForce RTX 3070".
GPUs tested: NVIDIA GeForce RTX 3070 (595.99.02), Linux
Tests
agc_structurizer,agc_structurizer_intersection_off,agc_structurizer_iteration_selection_off,agc_structurizer_forced_fallback,agc_shader_control_flow,agc_shader_control_flow_iteration_selection_off,agc_shader_control_flow_forced_fallbackandcontrol_flow_graphpass 5/5 on31cbb55d7and 3/3 on5d9dcc42d. They also passed 5/5 on unchangedmain.- The same 8 tests pass with
APS5_NO_PERF_DOM_INTERSECTION=1. ctest -R "^agc|shader|recompiler|control_flow" -E "agc_display_buffer_dcc|agc_driver_flip|agc_driver_lds_atomics64|display|flip" -j1 --timeout 60: 596/598 pass.agc_driver_stencil_ref_exportis skipped (noVK_EXT_shader_stencil_export), andagc_driver_store_over_untrackedfails as it does on unchangedmainhere (#6385).
Timing. The test executables were run directly, 20 times each, on an idle machine (load average about 1.5), alternating between the default and APS5_NO_PERF_DOM_INTERSECTION=1. These are wall times of the whole process, startup included:
| Executable | bitsets (default) | vectors (APS5_NO_PERF_DOM_INTERSECTION=1) |
|---|---|---|
agc_structurizer_tests |
median 558 ms (554-567) | median 596 ms (585-620) |
agc_shader_control_flow_tests |
median 111 ms (109-123) | median 114 ms (113-119) |
So the bitsets are about 6% faster on the structurizer test programs, and the difference is within noise on the control-flow ones. These are test CFGs, not a title's shaders, so this does not answer the question about large real programs or about the O(n²) memory.
Written with AI assistance (Claude Code). I ran the tests above on my machine.
|
Checked the bitset paths against the vector ones (initial sets, visit order, blocks without predecessors/successors, unreachable blocks, sorted output); the structurizer and control-flow tests pass here, and the measured ~6% on the structurizer tests is confirmed by jotitadev's run. Merging. |
What
The shader structurizer recomputes dominators and post-dominators after every control-flow rewrite. Each block started as a vector of every block id, and every iteration copied and intersected those vectors. The same fixed point now runs on bitsets and still writes the sorted id lists the rest of the compiler reads.
APS5_NO_PERF_DOM_INTERSECTION=1keeps the previous vector intersection.iterationSelectionMergestays on vectors whenAPS5_NO_PERF_ITERATION_SELECTION=1.Tested
Not tested. This machine has no MinGW GCC 15.2 toolchain, so
agc_structurizer,agc_structurizer_intersection_off,agc_structurizer_iteration_selection_off,agc_shader_control_flowandagc_shader_control_flow_iteration_selection_offwere not run.python tools/check_conventions.py --base origin/mainpassed.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)Made with Cursor