fix(vk): search the storage buffer tables in a loop on Intel #2513

Merged
thomas-montano merged 2 commits from fix/intel-gm-table-loop into main 2026-11-11 22:20:30 +00:00
thomas-montano commented 2026-11-11 21:25:13 +00:00 (Migrated from github.com)

On Intel GPUs under Mesa, the game's culling programs never finish compiling, and Intel builds compute pipelines one at a time (issue 496), so every compute build queued behind them waits for good. In Kokiri Forest on an Iris Xe that leaves the forest grey, with no foliage or light, and the rocks and doors the mesh fallback draws stay white.

Part of #999 and #1001

Why it happens

A global memory access the storage buffer pass could not bind is translated into a search of the storage buffer tables: four tables of 16 entries, about 70 entries in all, each written out in DefineGlobalMemoryFunctions. These helpers are marked DontInline for NVIDIA, but Mesa inlines every function anyway. The foliage culling program 53caac5f has 46 global accesses, so it ends up with over 100k instructions. At the 32 lanes compute is pinned to, Intel's compiler (brw, register allocation at SIMD32) did not finish it in 5+ minutes.

What changed

  • spirv_emit_context.cpp: when global_table_loop is set, each table is searched in a loop the driver is asked not to unroll (DontUnroll). The loop runs the same test as the written-out form, entry by entry: the same constant-buffer words, the same masked address and bounds, and the first match taken. Its counter is the same in every invocation that runs it, so this is dynamic indexing, not non-uniform, and modules with the loop declare StorageBufferArrayDynamicIndexing. The CAS loops' callers open their own block and keep the written-out form.
  • vk_coopmat.cpp: table_loop_wanted() turns it on for Intel's Mesa driver (VK_DRIVER_ID_INTEL_OPEN_SOURCE_MESA), only where the device has shaderStorageBufferArrayDynamicIndexing. The cause is Mesa's, so Intel's Windows driver, whose compiler is not Mesa's, keeps the written-out form until it has been tried. RX_VK_GM_LOOP overrides the default on any GPU, read as every switch is (env_value_off).
  • vk_shader.cpp: translator_stamp() takes the switch (flag bit 8). Programs cached in one form are not used in the other. The bit is 0 everywhere the loop is off, so on other GPUs the SPIR-V, the stamp and the cache are unchanged.
  • docs/development.md: a row for RX_VK_GM_LOOP.

The effect on the four table-heavy programs: 53caac5f's SPIR-V halves (998 KB to 448 KB), and each now builds in 12-15 s instead of not finishing.

Tests

  • tests/test_global_table_loop.py (new):
    • compiles table_loop_wanted and runs it for Intel's Mesa and Windows drivers, NVIDIA and AMD, with and without dynamic indexing and RX_VK_GM_LOOP (1, true, 0);
    • checks that translator_stamp takes the switch;
    • checks that the loop reads the words the written-out form reads (cbuf_offset / 8 + 2k for every offset and entry) and runs the same bounds test.
  • tests/global_load_16_ir.cpp: the global load fixture emits one module whose storage buffer table is searched in the loop, so the SPIR-V fixtures job runs spirv-val on the loop's form (its counter, the return inside its body).
  • The checks that read the same sources pass: python -I -B tests/run_all.py global_table_loop bake_mimo amd_fragment_warp bake_skeleton bake_ds cas_loop_ceiling global_atomic_add_f32 mesh_point_topology prepared_stamps recorder_sync program_cache_format program_stamp_knobs gave 12 passed on Linux. test_mesh_unreachable_outputs was skipped (no spirv-val here).

Tested in the game

Iris Xe (Raptor Lake-P, i9-13900H, 32 GB), Mesa ANV 26.2.4, Bazzite. Kokiri Forest, walking out of Link's house:

  • Before: 53caac5f still compiling after 5+ minutes, and every compute build queued behind it waiting. The forest stays grey and the mesh-drawn rocks, doors and trees stay white.
  • After: every compute program the forest dispatches builds and runs, and the forest gets its foliage and light. With everything built: about 17 fps in the forest, 52 ms of GPU time per frame (RX_VK_PASS_TIMING=2), 20 ms of it the game's own dispatches. That work was never run before, so earlier frame rates in this scene are not comparable.

Once the culling builds, the mesh fallback's colour draws still wait for their own (slow) builds. Until then the grass shows as sky-coloured holes, which bloom blows up into specks. #2382 holds those draws' depth while their colour programs build.

Not tested:

  • Other Intel GPUs (Arc, UHD).
  • Windows: untested for now. Intel's Windows driver keeps the written-out form; RX_VK_GM_LOOP=1 turns the loop on there.
  • GPUs where RX_VK_GM_LOOP=1 is set by hand.
  • Intel users' program caches are rebuilt once, because the stamp changes.

Overlap: #2306 also edits DefineGlobalMemoryFunctions (the float atomics). The code merges cleanly with it; only the docs/development.md row conflicts.

On Intel GPUs under Mesa, the game's culling programs never finish compiling, and Intel builds compute pipelines one at a time (issue 496), so every compute build queued behind them waits for good. In Kokiri Forest on an Iris Xe that leaves the forest grey, with no foliage or light, and the rocks and doors the mesh fallback draws stay white. Part of #999 and #1001 ## Why it happens A global memory access the storage buffer pass could not bind is translated into a search of the storage buffer tables: four tables of 16 entries, about 70 entries in all, each written out in `DefineGlobalMemoryFunctions`. These helpers are marked `DontInline` for NVIDIA, but Mesa inlines every function anyway. The foliage culling program 53caac5f has 46 global accesses, so it ends up with over 100k instructions. At the 32 lanes compute is pinned to, Intel's compiler (`brw`, register allocation at SIMD32) did not finish it in 5+ minutes. ## What changed - `spirv_emit_context.cpp`: when `global_table_loop` is set, each table is searched in a loop the driver is asked not to unroll (`DontUnroll`). The loop runs the same test as the written-out form, entry by entry: the same constant-buffer words, the same masked address and bounds, and the first match taken. Its counter is the same in every invocation that runs it, so this is dynamic indexing, not non-uniform, and modules with the loop declare `StorageBufferArrayDynamicIndexing`. The CAS loops' callers open their own block and keep the written-out form. - `vk_coopmat.cpp`: `table_loop_wanted()` turns it on for Intel's Mesa driver (`VK_DRIVER_ID_INTEL_OPEN_SOURCE_MESA`), only where the device has `shaderStorageBufferArrayDynamicIndexing`. The cause is Mesa's, so Intel's Windows driver, whose compiler is not Mesa's, keeps the written-out form until it has been tried. `RX_VK_GM_LOOP` overrides the default on any GPU, read as every switch is (`env_value_off`). - `vk_shader.cpp`: `translator_stamp()` takes the switch (flag bit 8). Programs cached in one form are not used in the other. The bit is 0 everywhere the loop is off, so on other GPUs the SPIR-V, the stamp and the cache are unchanged. - `docs/development.md`: a row for `RX_VK_GM_LOOP`. The effect on the four table-heavy programs: 53caac5f's SPIR-V halves (998 KB to 448 KB), and each now builds in 12-15 s instead of not finishing. ## Tests - `tests/test_global_table_loop.py` (new): - compiles `table_loop_wanted` and runs it for Intel's Mesa and Windows drivers, NVIDIA and AMD, with and without dynamic indexing and `RX_VK_GM_LOOP` (`1`, `true`, `0`); - checks that `translator_stamp` takes the switch; - checks that the loop reads the words the written-out form reads (cbuf_offset / 8 + 2k for every offset and entry) and runs the same bounds test. - `tests/global_load_16_ir.cpp`: the global load fixture emits one module whose storage buffer table is searched in the loop, so the SPIR-V fixtures job runs `spirv-val` on the loop's form (its counter, the return inside its body). - The checks that read the same sources pass: `python -I -B tests/run_all.py global_table_loop bake_mimo amd_fragment_warp bake_skeleton bake_ds cas_loop_ceiling global_atomic_add_f32 mesh_point_topology prepared_stamps recorder_sync program_cache_format program_stamp_knobs` gave 12 passed on Linux. `test_mesh_unreachable_outputs` was skipped (no `spirv-val` here). ## Tested in the game Iris Xe (Raptor Lake-P, i9-13900H, 32 GB), Mesa ANV 26.2.4, Bazzite. Kokiri Forest, walking out of Link's house: - Before: 53caac5f still compiling after 5+ minutes, and every compute build queued behind it waiting. The forest stays grey and the mesh-drawn rocks, doors and trees stay white. - After: every compute program the forest dispatches builds and runs, and the forest gets its foliage and light. With everything built: about 17 fps in the forest, 52 ms of GPU time per frame (`RX_VK_PASS_TIMING=2`), 20 ms of it the game's own dispatches. That work was never run before, so earlier frame rates in this scene are not comparable. Once the culling builds, the mesh fallback's colour draws still wait for their own (slow) builds. Until then the grass shows as sky-coloured holes, which bloom blows up into specks. #2382 holds those draws' depth while their colour programs build. **Not tested:** - Other Intel GPUs (Arc, UHD). - Windows: untested for now. Intel's Windows driver keeps the written-out form; `RX_VK_GM_LOOP=1` turns the loop on there. - GPUs where `RX_VK_GM_LOOP=1` is set by hand. - Intel users' program caches are rebuilt once, because the stamp changes. **Overlap:** #2306 also edits `DefineGlobalMemoryFunctions` (the float atomics). The code merges cleanly with it; only the `docs/development.md` row conflicts.
Joey-Magnus (Migrated from github.com) requested changes 2026-11-11 21:46:33 +00:00
Joey-Magnus (Migrated from github.com) left a comment

Good find. The loop runs the same test as the written-out form: the same words (cbuf_offset/8 + 2k), the same masked bounds, and it returns on the first match. Stamp bit 8 keeps the caches apart, and NVIDIA and AMD SPIR-V is unchanged. shaderStorageBufferArrayDynamicIndexing is also enabled whenever it's supported, because vk_main.cpp passes the queried features into vkCreateDevice.

  • should fix gpu/vk/vk_coopmat.cpp:349 uses std::atoi(env) != 0. The convention is env_value_off/env_on; with atoi, RX_VK_GM_LOOP=true turns the loop off.
  • should fix The cause is Mesa's: it inlines functions marked DontInline. But the switch keys on vendor 0x8086, so Intel's Windows driver gets the loop too, untested. dev.driver_id == VK_DRIVER_ID_INTEL_OPEN_SOURCE_MESA matches what was tested; widen it once Windows has been tried.
  • The loop form never reaches spirv-val in CI: the fixtures job runs with the switch off. One fixture validated with RX_VK_GM_LOOP=1 would cover the return inside the loop body.
Good find. The loop runs the same test as the written-out form: the same words (`cbuf_offset/8 + 2k`), the same masked bounds, and it returns on the first match. Stamp bit 8 keeps the caches apart, and NVIDIA and AMD SPIR-V is unchanged. `shaderStorageBufferArrayDynamicIndexing` is also enabled whenever it's supported, because `vk_main.cpp` passes the queried features into `vkCreateDevice`. - **should fix** `gpu/vk/vk_coopmat.cpp:349` uses `std::atoi(env) != 0`. The convention is `env_value_off`/`env_on`; with `atoi`, `RX_VK_GM_LOOP=true` turns the loop off. - **should fix** The cause is Mesa's: it inlines functions marked `DontInline`. But the switch keys on vendor 0x8086, so Intel's Windows driver gets the loop too, untested. `dev.driver_id == VK_DRIVER_ID_INTEL_OPEN_SOURCE_MESA` matches what was tested; widen it once Windows has been tried. - The loop form never reaches spirv-val in CI: the fixtures job runs with the switch off. One fixture validated with `RX_VK_GM_LOOP=1` would cover the return inside the loop body.
hckrman101 (Migrated from github.com) approved these changes 2026-11-11 22:18:43 +00:00
hckrman101 (Migrated from github.com) left a comment

Looks good. The second commit fixes all three of @Joey-Magnus's points. The switch now keys on VK_DRIVER_ID_INTEL_OPEN_SOURCE_MESA, so Intel's Windows driver keeps the written-out form. RX_VK_GM_LOOP goes through env_value_off, and the test covers true. The new check_table_loop fixture puts the loop form through spirv-val in CI. I checked the fixtures job log: the module is saved into global-load-16/modules and validated with the others.

I went through the loop against the written-out form. It reads the same words (cbuf_offset / 8 + 2k and the next one), masks the address the same way, applies the same size-0 rule and cap from OpArrayLength, and returns on the first match. The control flow is structured: the check block's conditional branch is a loop break, so it needs no merge, and the then-block ends in the callback's return. The counter's OpVariable is emitted right after the function's first label, and only when define_body opens that label itself, so the CAS callers are left alone. The k-indexed descriptor access is dynamically uniform, and the module declares StorageBufferArrayDynamicIndexing. The switch requires that feature, and the device enables it whenever it is supported. Stamp bit 8 keeps the two forms' caches apart. On other drivers the bit is 0, so their SPIR-V and caches don't change.

CI is green, and the PR merges cleanly with main. The only overlap is the docs/development.md row against #2306, as the description says. One small doc nit inline.

Looks good. The second commit fixes all three of @Joey-Magnus's points. The switch now keys on `VK_DRIVER_ID_INTEL_OPEN_SOURCE_MESA`, so Intel's Windows driver keeps the written-out form. `RX_VK_GM_LOOP` goes through `env_value_off`, and the test covers `true`. The new `check_table_loop` fixture puts the loop form through spirv-val in CI. I checked the fixtures job log: the module is saved into `global-load-16/modules` and validated with the others. I went through the loop against the written-out form. It reads the same words (`cbuf_offset / 8 + 2k` and the next one), masks the address the same way, applies the same size-0 rule and cap from `OpArrayLength`, and returns on the first match. The control flow is structured: the `check` block's conditional branch is a loop break, so it needs no merge, and the then-block ends in the callback's return. The counter's `OpVariable` is emitted right after the function's first label, and only when `define_body` opens that label itself, so the CAS callers are left alone. The k-indexed descriptor access is dynamically uniform, and the module declares `StorageBufferArrayDynamicIndexing`. The switch requires that feature, and the device enables it whenever it is supported. Stamp bit 8 keeps the two forms' caches apart. On other drivers the bit is 0, so their SPIR-V and caches don't change. CI is green, and the PR merges cleanly with main. The only overlap is the `docs/development.md` row against #2306, as the description says. One small doc nit inline.
@ -157,6 +157,7 @@ case-sensitive, and an unknown one is ignored.
| `RX_VK_PRECOMPILE=0` or `1` | The shader preparation before the title off or on (on in builds made with `RX_RELEASE=1`, as the builders' are) |
| `RX_VK_TASK_MESH_FALLBACK=0` or `1` | Task + mesh draws through the compute fallback (1) or the driver's mesh shaders (0). By default the fallback is on for AMD's Navi 21 and Navi 22 on Windows (RX 6800, 6800 XT, 6900 XT, 6700 XT), where the native path timed out the driver (issue 996) |
| `RX_VK_DRIVER_FAULTS=0` or `2` | 0: programs the driver's compiler faulted on are asked of it again. By default they are left out for the rest of the run and, through `hookshot_driver_faults.txt` beside the executable, on later runs with the same driver, and on AMD one program's builds go to the driver one at a time (issue 996). 2: one at a time on every driver |
| `RX_VK_GM_LOOP=0` or `1` | The storage buffer tables of a global memory access searched in a loop (1) or written out entry by entry (0). On by default on Intel, whose compiler did not finish the culling programs with the entries written out |
hckrman101 (Migrated from github.com) commented 2026-11-11 22:18:43 +00:00

nit: "On by default on Intel" now reads broader than the code. It is on by default for Intel's Mesa driver (Linux) only, and the Windows driver keeps the written-out form unless RX_VK_GM_LOOP=1 is set.

nit: "On by default on Intel" now reads broader than the code. It is on by default for Intel's Mesa driver (Linux) only, and the Windows driver keeps the written-out form unless `RX_VK_GM_LOOP=1` is set.
Commenting is not possible because the repository is archived.
No description provided.