Skip to content

perf(recompiler): select guest memory loads instead of branching around them - #4048

Merged
oneandonlydean merged 6 commits into
boykopovar:mainfrom
Mouad-Scriptz-0:perf/recompiler-select-load-guards
Oct 18, 2026
Merged

oneandonlydean merged 6 commits into
boykopovar:mainfrom
Mouad-Scriptz-0:perf/recompiler-select-load-guards

Conversation

@Mouad-Scriptz-0

@Mouad-Scriptz-0 Mouad-Scriptz-0 commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

What

Every guarded guest memory load in the SPIR-V backend (LoadWord, LoadSubword, LoadWideShared in SpirvMemoryInstructions.cpp, readBuffer and EmitRuntimeScalarBufferLoad in SpirvRuntimeBuffer.cpp) opened a selection per guard: one for the lane's exec bit and one for the bounds check, four blocks each, and once more per lane half for wave64 on a 32-wide host. Sony's PSP wrapper (Killzone: Liberation, PPSA11142) runs its GE vertex processing in a 3176-word merged wave64 program with 249 s_buffer_load, 229 LDS and 20 scratch accesses; it recompiled to 16,983 blocks, 11,274 of them dead OpUnreachable blocks left by the guards' arms, and 1,710 conditional branches in 484,166 words.

  • Scratch and direct storage-buffer loads compute the guard as before (active && index < length), perform the load either way and select 0 as the result. The load is pointed at a location no write can race with, so the extra access adds no data race (Vulkan memory model): scratch reads its own element 0; storage buffers (direct buffers, GDS) read element 0xfffffffe, which robustBufferAccess2 turns into a zero load with no memory access. The aligned high word of a buffer read re-reads the low word's own element.
  • VulkanDevice.cpp enables robustBufferAccess2 when the device has it, and the recompiler target carries the flag (cache key included). Without it those loads keep their selections as on main; LDS loads, stores, atomics, formatted and physical-address (BDA) accesses always do. LDS stays branched because NVIDIA predicates the branched load at no cost while the select form costs about 2.5 instructions per dword and splits ds_read_b128 into four loads (@karanvora2599's RTX 4060 counts below); the sentinel dword earlier revisions appended to the LDS array is gone with it, and the device-memory decision reserves the lock alone. agc_shader_replay --robust2 emits for such a device.
  • agc_guarded_memory_loads (new, built on @jweidenh's access tracer from the review) follows the emitted control flow for LDS word/subword/wide, GDS, coherent buffer and scratch loads in both device modes and records which guest elements each masked, in-range, out-of-range, invalid-descriptor and misaligned case reads; agc_lds_lock_budget and agc_driver_lds_lock_budget (@jweidenh) check the device-memory decision at the lock boundary.
  • On the same program, measured on main 7a84b22: 15,323 blocks / 1,295 conditional branches without robustBufferAccess2, 6,563 / 1,032 with it (main: 16,983 / 1,710; words 484,166 → 455,017).
  • NVIDIA (RTX 4060, driver 591.86, @karanvora2599): SASS instructions per guarded buffer_load_dword drop from 24.0 to 10.5, dwordx4 from 95.5 to 42.0, s_buffer_load_dword from 16.5 to 10.0; LDS reads stay at main's cost. agc_driver_flat_apertures_5 hangs that driver with this PR because the selected loads reach the BDA span-read pattern of Recompiler: NVIDIA hangs (device lost) when read_bda_span lanes split between in-place loads and the fallback; branch and commit ready #3291, and the split proposed there resolves it; CI does not run on NVIDIA.

The title still stalls on this program on Intel's Mesa driver for a separate reason, Mesa's register allocator spill loop on the wave64-sized instruction stream (every sample of the busy thread is in ra_get_best_spill_node/spill_reg; reported to Mesa), so this does not make PPSA11142 playable on that GPU by itself. It is a generic saving for every shader with guarded loads.

Tested

Fedora 44 (kernel 7.2.8, GCC 16.2.1, Ninja, Release, BUILD_TESTING=ON, Intel Arc Meteor Lake iGPU, Mesa 26.2.3, which exposes robustBufferAccess2), branch rebased on main 2cf23fc (2026-10-13; the #3291 span-read split is on main through #4759 and #5022, so the NVIDIA flat_apertures_5 hang karanvora2599 saw has its fix underneath this branch now; the debt entry lives in docs/dev/debt/shader.md): agc_guarded_memory_loads, agc_shader_memory, agc_shader_disk_cache, agc_vector_window, agc_driver_buffer_load_store, agc_driver_buffer_unaligned_base, agc_driver_lds_read_write, agc_driver_lds_beyond_device_limit, agc_driver_gds_append_consume, agc_driver_flat_loads, agc_driver_relative_indexing and agc_driver_scalar_relative_indexing pass. Before the correction, the 130-test memory subset (ctest -R 'lds|scratch|buffer|flat|global|scalar|relative|dpp16|shared|load|store|spirv|memory|aperture|wave64|masked|interp') passed with 1 skip. tools/check_conventions.py --base origin/main and git diff --check pass. Block and word counts from agc_shader_replay --spv on the dumped draw. Not tested on Windows.

Host recompile time of the PPSA11142 shader dumps on this machine (agc_shader_replay, best of 3 wall-clock runs per request, process start included; SPIR-V emission only, the driver's pipeline compile is not part of it): all 15 requests 699 ms on main (2a5eca1), 672 ms with this PR, 643 ms with --robust2; the 478,843-word program alone 296 ms on main, 253 ms with this PR, 243 ms with --robust2. Frame time with robustBufferAccess2 on and off still owed: the device enables it for every pipeline, so it needs a title capture per the performance form, which is queued behind a Minecraft start-up hang I'm bisecting.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: PPSA11142, PPSA17221 (no Game bring-up discussion yet)
  • AI-assisted: yes (Claude Code)

🤖 Generated with Claude Code

@github-actions github-actions Bot added shader core/shader docs labels Oct 11, 2026
@jweidenh

Copy link
Copy Markdown
Contributor

I found an unsafe extra-read case in head 13918b235d11157c3a6cb7e23f04fdeaabe22001 and prepared a tested correction.

For an aligned coherent buffer load at byte offset 8, the emitted path reads word 2 and performs a Volatile load from word 0 for the unused high half. Selecting zero afterward does not remove that access. Masked or out-of-range LDS/GDS/direct-buffer reads can likewise reach element zero. A concurrent write there can create a data race absent from the guarded program; addressability and robust access do not establish race freedom. See the Vulkan memory model.

The correction preserves index/result selects for invocation-private Function scratch, restores execution guards for shared reads and direct-buffer bounds, and loads the high word only for an unaligned base. It also updates TechnicalDebt to remove compile-time figures that no longer describe the narrowed optimization. Compile-time impact of the retained guards has not been measured.

Patch: test and correction together. The two commits are the regression and the fix, on jweidenh:codex/guarded-shader-loads.

The CPU regression follows concrete control-flow paths in emitted SPIR-V and records guest-memory load indices. On the test-only baseline, all six expected failure groups reproduced: LDS word/subword/wide, GDS word, coherent buffer bounds, and aligned high-word access. The last reported guest reads: 2 0; expected: 2. Private scratch remained a passing control. The baseline was not dispatched on the GPU.

Windows 11, RTX 5090, NVIDIA driver 617.42, Release build: all seven candidate CTests passed without skips, with the six native controls selecting the RTX 5090. Full libs build and local conventions checks also passed. Shader caching was disabled and each case used fresh cache directories. Times below are test wall times, not performance comparisons.

CTest Result Time
agc_driver_buffer_load_store pass 5.543 s
agc_driver_buffer_unaligned_base pass 1.265 s
agc_driver_gds_append_consume pass 0.914 s
agc_driver_lds_beyond_device_limit pass 1.935 s
agc_driver_lds_mixed_width_atomics pass 1.149 s
agc_driver_lds_read_write pass 2.991 s
agc_guarded_memory_loads pass 1.012 s

The seven CPU groups all pass with the fix, including active/inactive and bounds cases, conditional high-word reads, Volatile preservation, and an explicit Function-storage check for scratch. Baseline and candidate use identical test files.

Windows build dependency: both arms include only the reviewed Socket.cpp guard from #4021 at 121b16f. Actual validation pins are baseline 283ad65 and candidate 33d462e; each differs from its published shader revision only in that build fix. The proposed correction does not include it.

Limits: scalar-buffer lowering is restored but has no dedicated case in the new CPU fixture. Non-atomic GDS reads have CPU access-trace coverage; the native GDS test covers append/consume atomics. Linux was not rerun because the local runner still lacks the required SDL audio development packages. No retail-title crash, startup latency, shader compile-time reduction, or FPS improvement has been demonstrated.

AI-assisted contribution.

@Mouad-Scriptz-0
Mouad-Scriptz-0 force-pushed the perf/recompiler-select-load-guards branch from 13918b2 to 70b27ed Compare October 11, 2026 23:14
@Mouad-Scriptz-0

Copy link
Copy Markdown
Contributor Author

Thanks, the extra access is real and the memory model's rule is on your side: a load that the guarded program would not perform is an access, whatever happens to its value. Rather than narrow the selects to scratch, 70b27ed5 keeps them and points every selected load at a location that no write can race with:

  • scratch keeps element 0 (invocation-private);
  • LDS in workgroup or function memory reads a sentinel dword appended after the program's LDS (and after the lock dword), which no bounds-checked write reaches;
  • storage buffers (direct buffers, GDS) read element 0xfffffffe, which robustBufferAccess2 turns into a zero load with no memory access; VulkanDevice.cpp now enables robustBufferAccess2 when the device has it and the recompiler target carries the flag (cache key and agc_shader_disk_cache updated). Without it, and for LDS kept in device memory, those loads keep their selections exactly as on main;
  • the aligned high word of a buffer read re-reads the low word's own element instead of word 0, so the misaligned case touches nothing new, and the Volatile operand is kept.

Your GuardedMemoryLoads.cpp tracer is in the branch as agc_guarded_memory_loads (thanks, it is a good fixture), run in both device modes: with robustBufferAccess2 the masked and out-of-range reads record the sentinel (LDS) or 0xfffffffe (GDS, buffers, including the invalid-descriptor and misaligned cases), without it they record no read, and scratch records 0 either way. agc_shader_replay gained --robust2 to emit for such a device.

On the dumped PPSA11142 program, now on current main with #3856: 14,667 blocks / 1,131 conditional branches without robustBufferAccess2, 5,907 / 868 with it (main: 16,983 / 1,710). Tested on Fedora 44 / Intel Arc Meteor Lake (Mesa 26.2.3, which exposes robustBufferAccess2): agc_guarded_memory_loads, agc_shader_memory, agc_shader_disk_cache, agc_vector_window, agc_driver_buffer_load_store, agc_driver_buffer_unaligned_base, agc_driver_lds_read_write, agc_driver_lds_beyond_device_limit, agc_driver_gds_append_consume, agc_driver_flat_loads, agc_driver_relative_indexing and agc_driver_scalar_relative_indexing pass; conventions and git diff --check pass. Not tested on Windows.

AI-assisted: yes (Claude Code).

@jweidenh

Copy link
Copy Markdown
Contributor

I reviewed and tested the revised 70b27ed5 implementation on Windows 11 / RTX 5090. The sentinel strategy addresses the extra shared reads from my previous report, but I found a compute LDS budgeting boundary that still needs correction.

EnsureLdsStorage now declares guest LDS + a lock dword when required + the sentinel dword. DeviceMemoryLdsBytes still reserves only four bytes. A shader using the 64-bit LDS lock can therefore remain in Workgroup storage while declaring four bytes beyond maxWorkgroupSharedMemoryBytes.

The correction reserves eight bytes in the runtime and diagnostic recompilation decisions. It preserves your load-selection/robustness strategy and the existing device-memory fallback layout. This is conservative worst-case accounting, consistent with the existing fixed-overhead approach. The existing source-content cache version automatically invalidates old compiled artifacts.

Patch: tests and correction, on jweidenh:codex/guarded-load-sentinel-budget. The test-only baseline is 00e3a14ec; e638f90b contains only the two production budget-line changes. Test files and registrations are identical between both arms.

Negative control: the CPU fixture recompiles a small locked-LDS shader with 64 bytes of guest LDS and limits of 64, 68 and 72 bytes, for wave32 and wave64. On the test-only baseline, exactly the two 68-byte cases fail with declared 72 Workgroup bytes exceeds limit 68; the other four controls pass. All six pass with the correction. No baseline shader was dispatched on the GPU.

Native boundary proof: the new mode of the existing mixed-width-atomics test reads the actual device target. On this RTX 5090 / driver 617.42 it reports robustBufferAccess2=1 and a 49,152-byte shared-memory limit. Before dispatch it verifies the emitted allocation, then checks results across all 256 lanes:

  • Guest LDS 49,148 bytes (limit minus four): zero Workgroup bytes, device-backed stride 12,288 dwords; readback passes.
  • Guest LDS 49,144 bytes (limit minus eight): exactly 49,152 Workgroup bytes including lock and sentinel, no device-backed LDS; readback passes.

Windows results: Release, supported WinLibs GCC 15.2.0 r7, full libs target built. Thirteen of fourteen selected CTests passed; one executable was blocked before process creation by the machine's existing Windows Code Integrity signing policy. That test is not run, not a pass. The policy was not changed and the blocked test was not retried. All nine completed native cases selected the RTX 5090. The table gives test wall times, not performance measurements.

CTest Result Time
agc_driver_buffer_unaligned_base pass 3.461 s
agc_driver_flat_loads pass 0.993 s
agc_driver_gds_append_consume pass 0.918 s
agc_driver_lds_beyond_device_limit pass 2.235 s
agc_driver_lds_mixed_width_atomics pass 1.134 s
agc_driver_lds_read_write pass 3.415 s
agc_driver_lds_sentinel_budget pass 0.679 s
agc_driver_relative_indexing pass 1.934 s
agc_driver_scalar_relative_indexing pass 0.864 s
agc_guarded_memory_loads pass 1.234 s
agc_lds_sentinel_budget pass 1.179 s
agc_shader_disk_cache pass 1.413 s
agc_shader_memory pass 2.537 s
agc_driver_buffer_load_store not run: Code Integrity policy —

The revised access tracer passed all seven groups, which exercise robustness2 disabled and enabled internally. The native boundary test explicitly verifies enabled robustness2; the other native cases identify the GPU without their own feature banner. Tests use isolated cache locations; the disk-cache test intentionally enables its own temporary cache. Source, test inventory, raw JUnit/logs, and allocation/readback evidence were independently audited. Conventions and git diff --check pass. The overall fourteen-test validation remains incomplete because of the policy-blocked executable.

Build dependency: although #4021 has now merged, the tested PR base predates it. Both arms add exactly its reviewed Socket.cpp guard at 121b16fb4bbd47ff4ce9302c40631bab858e3e71, separately from this correction. Actual Windows pins are baseline e85cfad9 and candidate 08522f5f.

Limits: Linux was not rerun because the local runner still lacks SDL audio development packages. The unchanged historical agc_vector_window fixture was excluded because its missing observable output had already caused a known failure; it is not counted as passing. This correction covers compute budgeting; overall mesh Workgroup budgeting and shader-request serialization of the new robustness flag are outside it. The diagnostic recompilation budget change was source-reviewed, with no separate CLI execution test. No retail title, startup latency, shader compilation-time improvement or FPS improvement has been demonstrated.

AI-assisted contribution.

@Mouad-Scriptz-0

Copy link
Copy Markdown
Contributor Author

Right, the fallback decision still budgeted the lock alone. Your three commits are on the branch as they are (15168cc0, bcf90fa9, d6f57526, your authorship): DeviceMemoryLdsBytes and the deserializer now reserve eight bytes for the lock and the sentinel, and the two tests are registered.

On Fedora 44 / Intel Arc Meteor Lake (Mesa 26.2.3, robustBufferAccess2=1, 49,152-byte shared-memory limit, the same as your 5090): agc_lds_sentinel_budget (the six CPU boundary cases), agc_driver_lds_sentinel_budget (both native boundaries, readback checked across all lanes), agc_driver_lds_mixed_width_atomics, agc_guarded_memory_loads and agc_shader_memory pass; tools/check_conventions.py --base origin/main and git diff --check pass. Thanks for the boundary proof on the 5090 as well.

AI-assisted: yes (Claude Code).

@karanvora2599

Copy link
Copy Markdown
Contributor

Measurements on NVIDIA for d6f5752 merged onto main d146ec1. Setup: RTX 4060 Laptop (Ada, driver 591.86), Windows 11, MinGW-w64 GCC 15.2.0, Release.

Code size. SASS instructions per guest instruction come from the APS5_PIPELINE_STATS binary size: a wave32 compute program with 16 of the instruction, minus the same program with the instruction replaced by moves (FLOAT_MODE 0xc0).

Instruction main this PR
buffer_load_dword, buffer_load_dword glc 24.0 10.5
buffer_load_ubyte, buffer_load_ushort 25.0 11.5
buffer_load_dwordx4 95.5 42.0
s_buffer_load_dword, and per dword of x4/x16 16.5 10.0
ds_read_b32 0.5 2.5
ds_read_b64 0.5 5.5
ds_read_b128 1.0 11.5

Stores, atomics and buffer_load_format_xyzw move by 1 at most.

Why the LDS reads grow.

  • On main, a guarded LDS read is a branch around the load. NVIDIA's compiler turns that into a predicated LDS, which is nearly free, and ds_read_b128 stays one 128-bit load.
  • With this PR, each dword computes its own bounds check, selects its index against the sentinel, loads, and selects the value. That costs about 2.5 instructions per dword. The four loads of a ds_read_b128 can also no longer be merged into one.

With LDS reads left branched. I made LoadsSelectable return false for ResourceKind::Lds. LDS then takes the EmitValueOrZeroIfCondition path that LDS in device memory already uses. Results:

  • RTX 4060:
    • ds_read_b32 is 0.0, ds_read_b64 0.5 and ds_read_b128 1.0, which is main's cost or less.
    • Every buffer and scalar-buffer number in the table stays as with this PR.
  • Intel UHD Graphics (Raptor Lake, driver 101.6790), GEN instructions from the same statistics: ds_read_b32 drops from 20.3 to 15.3. But ds_read_b64 rises from 31.6 to 34.1, and ds_read_b128 from 66.1 to 82.6. So on Intel the static count favours the select for wide reads. I haven't measured run time on either GPU.
  • Tests on the RTX 4060:
    • Pass: every agc_driver_lds_* execution test, agc_lds_sentinel_budget, agc_driver_lds_sentinel_budget and agc_shader_memory.
    • agc_guarded_memory_loads fails only its three "LDS … reads its sentinel" checks, which pin the select form.

If LDS stays branched, no LDS load reads the sentinel any more. The sentinel dword and the eight-byte budget from d6f5752 would then not be needed for LDS.

Tests with the PR as it is, on the RTX 4060.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 12, 2026
Mouad-Scriptz-0 pushed a commit to Mouad-Scriptz-0/AnyPS5 that referenced this pull request Oct 12, 2026
NVIDIA's compiler predicates a branched LDS load at no cost, while the
selected form costs about 2.5 instructions per dword and splits a
ds_read_b128 into four loads (RTX 4060 measurements by @karanvora2599 in
boykopovar#4048); Intel's static counts favour the select only for wide reads. LDS
loads therefore keep the branch of main, in workgroup, function and device
memory alike, and the sentinel dword that only they read goes away: the LDS
array is the program's dwords plus the lock again, and the device-memory
decision reserves the lock alone. The sentinel budget tests become lock
budget tests at that boundary, and the guarded-load test pins the branch.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@Mouad-Scriptz-0
Mouad-Scriptz-0 force-pushed the perf/recompiler-select-load-guards branch from d6f5752 to 5d18a4d Compare October 12, 2026 09:08
@Mouad-Scriptz-0

Copy link
Copy Markdown
Contributor Author

Thanks for the SASS and GEN counts; they settle it. LDS loads keep the branch of main again, everywhere (workgroup, function and device memory), and the sentinel dword goes with them: nothing reads it once LDS is branched, so the LDS array is the program's dwords plus the lock again and the device-memory decision reserves the lock alone (DeviceMemoryLdsBytes and the deserializer back to 4 bytes). Buffer, scalar-buffer and scratch loads keep the select form your table shows halving. Commit 5d18a4d, on the branch rebased onto main c36fd0a (the conflicts were the new registrationModules target fields next to robustBufferAccess2).

  • LoadsSelectable returns false for ResourceKind::Lds, as in your experiment; GuardedElementIndex throws if LDS ever reaches it.
  • @jweidenh's agc_lds_sentinel_budget and agc_driver_lds_sentinel_budget become agc_lds_lock_budget and agc_driver_lds_lock_budget: the CPU cases now expect device memory below 68 bytes (16 dwords + lock) and a Workgroup allocation at and above it; the native cases check the exact fit at limit - 4 and the fallback at limit.
  • agc_guarded_memory_loads pins the branch for LDS word, subword and wide reads (masked and out-of-range cases read nothing; the x4 edge case reads element 15 only) and checks the LDS array declares exactly the program's dwords.
  • TechnicalDebt records why LDS stays branched, with your numbers and that run time was not measured on either GPU.

Counts on the same PPSA11142 program (agc_shader_replay --spv), measured on the previous base 7a84b22, this change against the sentinel version: without robustBufferAccess2 15,323 blocks / 1,295 conditional branches / 478,491 words (was 14,667 / 1,131), with it 6,563 / 1,032 / 455,017 (was 5,907 / 868 / 452,397); main is 16,983 / 1,710 / 484,166.

On the flat_apertures_5 hang: the description now notes that NVIDIA reaches the BDA span-read pattern of #3291 through the selected loads and that the split proposed there resolves it; thanks for isolating that.

Fedora 44 / Intel Arc Meteor Lake (Mesa 26.2.3, robustBufferAccess2=1): agc_guarded_memory_loads, agc_lds_lock_budget, agc_driver_lds_lock_budget, agc_driver_lds_mixed_width_atomics, agc_shader_memory pass; tools/check_conventions.py --base origin/main and git diff --check pass.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 12, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 12, 2026
Mouad-Scriptz-0 pushed a commit to Mouad-Scriptz-0/AnyPS5 that referenced this pull request Oct 12, 2026
NVIDIA's compiler predicates a branched LDS load at no cost, while the
selected form costs about 2.5 instructions per dword and splits a
ds_read_b128 into four loads (RTX 4060 measurements by @karanvora2599 in
boykopovar#4048); Intel's static counts favour the select only for wide reads. LDS
loads therefore keep the branch of main, in workgroup, function and device
memory alike, and the sentinel dword that only they read goes away: the LDS
array is the program's dwords plus the lock again, and the device-memory
decision reserves the lock alone. The sentinel budget tests become lock
budget tests at that boundary, and the guarded-load test pins the branch.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@ThatsLeo

Copy link
Copy Markdown
Contributor

@oneandonlydean asked for the frame time with robustBufferAccess2 on and off. Here it is on Intel, where the two differ.

Setup. Intel Arc (Meteor Lake iGPU), Mesa 26.2.3 ANV, Fedora 44 (kernel 7.2.10), GCC 16.2.1, Release.

frame p50 (ms) p90 (ms) GPU busy per present (ms)
main 675.5 / 681.7 698.2 / 707.0 479.9 / 480.3
this PR, robustBufferAccess2 on 567.1 / 588.7 726.9 / 768.2 405.9 / 423.9
this PR, off 688.4 / 692.8 715.5 / 717.9 492.8 / 487.4

With it on, GPU time drops about 14% and frame p50 about 15%. Off is within about 2% of main. The p90 of the "on" runs is higher, from a few slow frames in both captures.

One more data point, for the cost of the feature itself. Before finding this PR, I wrote a local variant of the same idea for readBuffer and EmitRuntimeScalarBufferLoad's direct path, without robustBufferAccess2:

  • the host index is clamped to element 0 when it is past the binding's OpArrayLength;
  • the result is selected instead of branched;
  • the misaligned high word is selected the same way.

On the same base and scene it gave 561.0 / 558.3 ms p50 and 378.1 / 356.1 ms GPU busy per present, which is lower than "on". The constant-buffer-heavy compute 0x2011050000 went from 0.59 to 0.25 ms per dispatch against main.

So on ANV, enabling robustBufferAccess2 seems to cost part of what the select form saves. That fits ANV bounds-checking every buffer access once the feature is on. I haven't isolated that cost on its own.

The clamped read has the problem your description avoids: element 0 can be written by another invocation while the discarded read happens, which is a data race under the memory model even though the value is never used. So I'm not proposing it as is; I'm only noting the cost of the feature on this driver.

{"commit":"1e6494b7ba94","dirty":false,"env":{"APS5_NO_LIBRARY_PREBUILD":"1","APS5_TRACE_FRAMES":"20:30"},"gpu":"Intel(R) Arc(tm) Graphics (MTL)","driver":"Intel open-source Mesa driver Mesa 26.2.4","os":"Linux 7.2.10-200.fc44.x86_64","resolution":"3840x2160","window":{"spec":"frames 20:30","first_present":20,"ms":20307.150},"presents":30,"frame_ms":{"p50":675.511,"p90":698.154,"p99":739.287,"max":739.287},"gpu_busy_ms_per_present":479.874,"worker_busy_pct":{"queue worker 0x0":97.095,"queue worker 0x20":96.645},"top_phases_ms_per_frame":{"Driver.Packet":1306.299,"Driver.Packet.memory_wait":638.944,"Driver.Packet.device_lock":550.439,"Driver.Present":306.284,"VideoOut.Flip":295.691,"VideoOut.Flip.present":295.623,"Driver.Present.render_fence_wait":293.110,"Vulkan.Retire":293.108},"trace_dropped":false,"trace":"aps5-trace-1e6494b7ba94-301749.json.gz"}
{"commit":"1e6494b7ba94","dirty":false,"env":{"APS5_NO_LIBRARY_PREBUILD":"1","APS5_TRACE_FRAMES":"20:30"},"gpu":"Intel(R) Arc(tm) Graphics (MTL)","driver":"Intel open-source Mesa driver Mesa 26.2.4","os":"Linux 7.2.10-200.fc44.x86_64","resolution":"3840x2160","window":{"spec":"frames 20:30","first_present":20,"ms":20573.828},"presents":30,"frame_ms":{"p50":681.725,"p90":707.036,"p99":1009.217,"max":1009.217},"gpu_busy_ms_per_present":480.334,"worker_busy_pct":{"queue worker 0x0":97.233,"queue worker 0x20":96.798},"top_phases_ms_per_frame":{"Driver.Packet":1325.293,"Driver.Packet.memory_wait":646.466,"Driver.Packet.device_lock":555.584,"Driver.Present":292.787,"VideoOut.Flip":282.517,"VideoOut.Flip.present":282.457,"Driver.Present.render_fence_wait":268.056,"Vulkan.Retire":268.053},"trace_dropped":false,"trace":"aps5-trace-1e6494b7ba94-302859.json.gz"}
{"commit":"d4658e3cf855","dirty":false,"env":{"APS5_NO_LIBRARY_PREBUILD":"1","APS5_TRACE_FRAMES":"20:30"},"gpu":"Intel(R) Arc(tm) Graphics (MTL)","driver":"Intel open-source Mesa driver Mesa 26.2.4","os":"Linux 7.2.10-200.fc44.x86_64","resolution":"3840x2160","window":{"spec":"frames 20:30","first_present":20,"ms":17402.436},"presents":30,"frame_ms":{"p50":561.000,"p90":721.866,"p99":787.124,"max":787.124},"gpu_busy_ms_per_present":378.149,"worker_busy_pct":{"queue worker 0x0":96.753,"queue worker 0x20":96.861},"top_phases_ms_per_frame":{"Driver.Packet":1118.858,"Driver.Packet.memory_wait":546.029,"Driver.Packet.device_lock":449.943,"Driver.Present":207.045,"VideoOut.Flip":201.325,"VideoOut.Flip.present":201.257,"Driver.Present.render_fence_wait":193.183,"Vulkan.Retire":193.180},"trace_dropped":false,"trace":"aps5-trace-d4658e3cf855-306110.json.gz"}
{"commit":"d4658e3cf855","dirty":false,"env":{"APS5_NO_LIBRARY_PREBUILD":"1","APS5_TRACE_FRAMES":"20:30"},"gpu":"Intel(R) Arc(tm) Graphics (MTL)","driver":"Intel open-source Mesa driver Mesa 26.2.4","os":"Linux 7.2.10-200.fc44.x86_64","resolution":"3840x2160","window":{"spec":"frames 20:30","first_present":20,"ms":16734.640},"presents":30,"frame_ms":{"p50":558.347,"p90":578.159,"p99":584.903,"max":584.903},"gpu_busy_ms_per_present":356.139,"worker_busy_pct":{"queue worker 0x0":95.978,"queue worker 0x20":96.740},"top_phases_ms_per_frame":{"Driver.Packet":1069.873,"Driver.Packet.memory_wait":524.596,"Driver.Packet.device_lock":249.578,"Driver.Packet.dispatch":237.079,"Driver.Dispatch":233.707,"Driver.Present":180.741,"VideoOut.Flip":174.622,"VideoOut.Flip.present":174.562},"trace_dropped":false,"trace":"aps5-trace-d4658e3cf855-307367.json.gz"}
{"commit":"9743a23404c2","dirty":false,"env":{"APS5_NO_LIBRARY_PREBUILD":"1","APS5_TRACE_FRAMES":"20:30"},"gpu":"Intel(R) Arc(tm) Graphics (MTL)","driver":"Intel open-source Mesa driver Mesa 26.2.4","os":"Linux 7.2.10-200.fc44.x86_64","resolution":"3840x2160","window":{"spec":"frames 20:30","first_present":20,"ms":17997.618},"presents":30,"frame_ms":{"p50":567.128,"p90":726.948,"p99":736.007,"max":736.007},"gpu_busy_ms_per_present":405.867,"worker_busy_pct":{"queue worker 0x0":97.140,"queue worker 0x20":95.513},"top_phases_ms_per_frame":{"Driver.Packet":1149.524,"Driver.Packet.memory_wait":555.957,"Driver.Packet.device_lock":470.873,"Driver.Present":223.954,"VideoOut.Flip":210.555,"VideoOut.Flip.present":210.494,"Driver.Present.render_fence_wait":210.308,"Vulkan.Retire":210.305},"trace_dropped":false,"trace":"aps5-trace-9743a23404c2-342753.json.gz"}
{"commit":"9743a23404c2","dirty":false,"env":{"APS5_NO_LIBRARY_PREBUILD":"1","APS5_TRACE_FRAMES":"20:30"},"gpu":"Intel(R) Arc(tm) Graphics (MTL)","driver":"Intel open-source Mesa driver Mesa 26.2.4","os":"Linux 7.2.10-200.fc44.x86_64","resolution":"3840x2160","window":{"spec":"frames 20:30","first_present":20,"ms":18693.821},"presents":30,"frame_ms":{"p50":588.671,"p90":768.204,"p99":832.084,"max":832.084},"gpu_busy_ms_per_present":423.872,"worker_busy_pct":{"queue worker 0x0":96.670,"queue worker 0x20":96.878},"top_phases_ms_per_frame":{"Driver.Packet":1201.091,"Driver.Packet.memory_wait":586.394,"Driver.Packet.dispatch":307.307,"Driver.Dispatch":302.502,"Driver.Present":251.347,"VideoOut.Flip":244.994,"VideoOut.Flip.present":244.926,"Driver.Packet.device_lock":244.535},"trace_dropped":false,"trace":"aps5-trace-9743a23404c2-343863.json.gz"}
{"commit":"2c6b31355f87","dirty":false,"env":{"APS5_NO_LIBRARY_PREBUILD":"1","APS5_TRACE_FRAMES":"20:30"},"gpu":"Intel(R) Arc(tm) Graphics (MTL)","driver":"Intel open-source Mesa driver Mesa 26.2.4","os":"Linux 7.2.10-200.fc44.x86_64","resolution":"3840x2160","window":{"spec":"frames 20:30","first_present":20,"ms":20708.339},"presents":30,"frame_ms":{"p50":688.350,"p90":715.498,"p99":729.206,"max":729.206},"gpu_busy_ms_per_present":492.828,"worker_busy_pct":{"queue worker 0x0":97.036,"queue worker 0x20":96.653},"top_phases_ms_per_frame":{"Driver.Packet":1333.377,"Driver.Packet.memory_wait":653.603,"Driver.Packet.dispatch":365.996,"Driver.Dispatch":363.418,"Driver.Present":310.819,"Driver.Dispatch.decode_key_device_setup":303.181,"VideoOut.Flip":300.327,"VideoOut.Flip.present":300.263},"trace_dropped":false,"trace":"aps5-trace-2c6b31355f87-347472.json.gz"}
{"commit":"2c6b31355f87","dirty":false,"env":{"APS5_NO_LIBRARY_PREBUILD":"1","APS5_TRACE_FRAMES":"20:30"},"gpu":"Intel(R) Arc(tm) Graphics (MTL)","driver":"Intel open-source Mesa driver Mesa 26.2.4","os":"Linux 7.2.10-200.fc44.x86_64","resolution":"3840x2160","window":{"spec":"frames 20:30","first_present":20,"ms":20736.501},"presents":30,"frame_ms":{"p50":692.767,"p90":717.917,"p99":725.210,"max":725.210},"gpu_busy_ms_per_present":487.353,"worker_busy_pct":{"queue worker 0x0":97.169,"queue worker 0x20":96.880},"top_phases_ms_per_frame":{"Driver.Packet":1336.580,"Driver.Packet.memory_wait":654.483,"Driver.Packet.device_lock":559.638,"Driver.Present":309.105,"VideoOut.Flip":299.186,"VideoOut.Flip.present":299.126,"Driver.Present.render_fence_wait":295.219,"Vulkan.Retire":295.215},"trace_dropped":false,"trace":"aps5-trace-2c6b31355f87-348717.json.gz"}

Traces, trace_digest.py output and the clamped-read diff: ThatsLeo:evidence/4048-stray (main/, robust2-on/, robust2-off/, clamped/).

AI-assisted: yes

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 16, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

@ThatsLeo's run answers the frame-time question on this head: with the feature on, GPU time drops by about 14% on ANV (p50 −15%), and with it off it's within 2% of main. What's left is the rebase: CacheKey, Recompiler, SpirvEmitter, ShaderReplay, the ShaderDiskCache test and the shader debt file conflict with main now. I'll rebase it onto current main. #6177 enables the same feature for stores and formatted accesses, so I'd like the enable and the cache key to land once, here, with #6177 building on it.

songanane and others added 6 commits October 17, 2026 12:18
…nd them

LDS, scratch and direct storage-buffer loads load element 0 when the lane is
inactive or the address is out of bounds and select 0 as the result, instead of
opening a selection per guard. Stores, atomics, formatted and physical-address
accesses keep their branches. A branch guard cost four blocks per lane half, and
the host compiler's time grows with the block count: PPSA11142's merged PSP
vertex program emitted 16,983 blocks and 1,710 conditional branches, 5,907 and
868 with the selects.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A load selected instead of branched still accesses memory, so its fallback
element must be one no write can race with: scratch keeps element 0, LDS in
workgroup or function memory reads a sentinel dword appended after the
program's LDS and lock, and storage buffers read element 0xfffffffe, which
robustBufferAccess2 turns into a zero load without a memory access. The
device enables robustBufferAccess2 when it has it and the target carries the
flag; without it, and for LDS in device memory, buffer loads keep their
selections. The aligned high word of a buffer read re-reads the low word's
element instead of word 0. agc_guarded_memory_loads traces the emitted loads
in both device modes.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
NVIDIA's compiler predicates a branched LDS load at no cost, while the
selected form costs about 2.5 instructions per dword and splits a
ds_read_b128 into four loads (RTX 4060 measurements by @karanvora2599 in
loads therefore keep the branch of main, in workgroup, function and device
memory alike, and the sentinel dword that only they read goes away: the LDS
array is the program's dwords plus the lock again, and the device-memory
decision reserves the lock alone. The sentinel budget tests become lock
budget tests at that boundary, and the guarded-load test pins the branch.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@oneandonlydean
oneandonlydean force-pushed the perf/recompiler-select-load-guards branch from 33f191d to 0a9ee36 Compare October 17, 2026 01:33
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Rebased onto main (resolutions: adjacent additions only); GPU lds tests 17/17.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 17, 2026
@oneandonlydean
oneandonlydean merged commit 63da333 into boykopovar:main Oct 18, 2026
8 checks passed
oneandonlydean pushed a commit that referenced this pull request Oct 18, 2026
Since #4048 enabled robustBufferAccess2, AMD's Windows driver draws
nothing in agc_driver_layered_volume_draw and agc_driver_layered_volume_vertex,
and the selected loads make the high half of agc_driver_subvector_loop's
wave64 skip its loop. Until the cause is found, that driver does not get
the feature, and its loads keep their selections.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Mouad-Scriptz-0

Copy link
Copy Markdown
Contributor Author

@ThatsLeo thanks, that closes the frame-time question @oneandonlydean asked for, and it points the same way as the SASS and GEN counts: the select form wins where the guard is paid per load. Dean, thanks for carrying the rebase and the merge.

On the clamped variant: the clamp to element 0 is the reason this PR reads element 0xfffffffe under robustBufferAccess2 instead. A read of element 0 races with any invocation that stores there, discarded value or not, which is the memory-model point in the description. So the variant is not a candidate as it is, but its GPU-busy numbers against the "on" build do suggest the feature costs something on ANV by itself, separate from the select.

One run would isolate that, if you have the setup still warm: main with the enable alone (state->robustBufferAccess2 = true and no select changes) on the same Stray window. If main plus the enable is slower than main, the cost is the driver's per-access check and not the select form, and the enable should become per driver or per access kind rather than unconditional. That matters for #6177 too, which extends the same enable to stores and formatted accesses. I will file a follow-up issue with the ANV numbers once that run exists, or now if you would rather track it separately.

@ThatsLeo

Copy link
Copy Markdown
Contributor

@Mouad-Scriptz-0 Here is the run you asked for. It isolates the enable, and it corrects what I suggested above: on ANV the feature costs nothing measurable by itself. The whole gain is the select form.

Setup.

  • Hardware and driver: Intel Arc (Meteor Lake), Mesa 26.2.4 ANV, Linux 7.2.10.
  • Scene: Stray, the same boot window (APS5_TRACE_FRAMES=20:30, 3840x2160).
  • Run settings: APS5_NO_LIBRARY_PREBUILD=1 (AGC: since #5875, a registration library prebuild racing the window attach segfaults in LinkPipelineFromLibraries (Stray PPSA02100, warm shader cache) #6805) and APS5_PLAYGO_UNLISTED_CHUNKS=absent in every run.
  • Build: main at this PR's merge, 63da333b7, with one local switch, in switch.diff, so all three variants share one build:
    • main: as merged. The device enables robustBufferAccess2 and the loads use the select form.
    • enable only: the device still enables robustBufferAccess2, but the recompiler target is told it is off, so the loads keep their selections as before this PR.
    • neither: the feature is not enabled and the loads keep their selections, which is the pre-PR behaviour.

What changed from my earlier runs. I found my first runs were not comparable: under this load the package reaches 97–99 °C and the GPU drops to 1550–1750 MHz despite the 2250 MHz minimum. So these are four interleaved rounds, each capture started below 55 °C.

GPU busy per present (ms), 4 runs mean frame p50 mean (ms)
main (enable + select) 341.8 / 345.6 / 354.8 / 322.7 341.2 473.1
enable only 459.5 / 465.6 / 467.1 / 457.8 462.5 583.5
neither 464.1 / 480.7 / 455.5 / 472.7 468.2 589.1
  • Enable alone against neither: −1.2% GPU time, inside the spread of the "neither" runs (sd 10.9 ms). No per-access cost shows up on ANV in this scene.
  • Select form on top of the enable: −26% GPU time.

That also means my clamped-variant numbers above should not be read as a cost of the feature. They came from runs where the thermal state wasn't controlled.

So from ANV there is no reason to make the enable per-driver. That leaves #6177 free to extend it here as planned. I can't speak for other drivers. 48b394d already keeps it off on AMD's proprietary driver, but for a different reason.

Traces, trace_digest.py output and the switch: ThatsLeo:evidence/4048-robust2-only (main/, dev/ = enable only, none/).

AI-assisted: yes

@Mouad-Scriptz-0

Copy link
Copy Markdown
Contributor Author

@ThatsLeo thanks for the controlled rounds. With the enable alone inside the spread of "neither", there is no per-access cost to track on ANV, so I'm not opening the follow-up issue I offered: it would have no claim to make. Recording the conclusion here instead: the enable can stay unconditional on ANV, the gain is the select form, and #6177 is free to extend the enable to stores and formatted accesses on this evidence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* build docs shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants