Repository navigation
fix(shader): share wave64 VGPRs past the private ones between the two halves - #7410
wachi35676 wants to merge 2 commits into
Conversation
|
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.
|
70e15b4 to
d662988
Compare
|
Tested on Windows 11, MinGW-w64 GCC 15.2.0, NVIDIA GeForce RTX 5090 (driver 617.42), Ryzen 9 9950X3D. Head
A note for other Windows testers: in a build folder under a long path (mine was a workspace path of about 150 characters) the build of I did not run the new test against AI-assisted: yes. Written with AI assistance and reviewed by me. |
d662988 to
e68ea55
Compare
e68ea55 to
455d68f
Compare
|
Rebased onto
AI-assisted: yes (Claude Code); results above were run on this machine. |
455d68f to
39b3e27
Compare
|
Rebased again, onto
AI-assisted: yes (Claude Code); results above were run on this machine. |
|
Rebase handoff — rebased onto current Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release, To pick it up: Head is |
|
One note on the handoff: an earlier full-suite run (on a stale build-tree state) showed |
|
@blawesom thanks for rebasing it and for the RX 9070 XT run. I compared AI-assisted: yes (Claude Code); results above were run on this machine. |
|
PS5LM Plus on an RDNA 3.5 iGPU: with this PR it gives the CPU backend's exact answer in all three configurations. Here the title isn't garbage on AMD. AMD Radeon 860M (
This is the same pattern you measured on the RTX 5070 Ti, with one difference: on this GPU The new tests ( A separate stop, not from this PR. In three cases, the first run of a configuration on a fresh build stopped about 11 s after start, before the server answered: AI-assisted: yes (Claude Code) |
39b3e27 to
5cea227
Compare
|
Second AMD-on-Linux (RADV) run, this time on RDNA3, after @blawesom's RX 9070 XT (RDNA4). Merged onto AMD Radeon RX 7900 XT (RADV NAVI31, gfx1100), Mesa 25.2.8, Linux Mint 22.3 (Ubuntu 24.04 base), kernel 7.0.0-38, GCC 13.3, Release. Full suite ( The new GPUs tested: AMD Radeon RX 7900 XT (RADV NAVI31, gfx1100), Mesa 25.2.8, Linux Mint 22.3 / kernel 7.0.0-38 AI-assisted: yes |
|
First AMD-on-Linux (RADV) run of this PR. Merged onto AMD Radeon RX 7900 XT (RADV NAVI31, gfx1100), Mesa 25.2.8, Linux Mint 22.3 (Ubuntu 24.04 base), kernel 7.0.0-38, GCC 13.3, Release. Full suite ( The PR's new ctest entry, This checks correctness only; I did not measure performance. GPUs tested: AMD Radeon RX 7900 XT (RADV NAVI31, gfx1100), Mesa 25.2.8, Linux Mint 22.3 / kernel 7.0.0-38 AI-assisted: yes |
a165137 to
691c101
Compare
|
Rebased onto Windows 11, WinLibs GCC 15.2.0:
Thanks @DavidAmsellem for the RX 7900 XT RADV run. AI-assisted: yes (Claude Code); results above were run on this machine. |
691c101 to
a3fe509
Compare
|
Rebased onto
AI-assisted: yes (Claude Code); results above were run on this machine. |
|
One more PS5LM Plus (PPSA99581) case this PR fixes, with a different model and a longer prompt: the flash-attention path with f16 K/V (the app's default Model: SmolLM2-360M-Instruct q8_0 from HuggingFaceTB's GGUF repo (Apache-2.0), from
AI-assisted: yes (Claude Code); results above were run on this machine. |
… halves A wave64 compute program declares COMPUTE_PGM_RSRC3.SHARED_VGPR_CNT blocks of eight wave-shared VGPRs right after its (COMPUTE_PGM_RSRC1.VGPRS + 1) * 4 private ones (RDNA2 ISA 3.6.5): lane N and lane N + 32 read and write one copy. ACO swaps the halves of a wave64 through two of them on GFX10 (emit_permlane64_shared_vgpr) and emulates a wave-wide ds_bpermute_b32 with them (emit_bpermute_shared_vgpr). The flash-attention reductions of llama.cpp's Vulkan backend reach the swap. The recompiler kept these VGPRs private, so each half read its own stale copy and the upper half's partial sums were dropped. The driver now decodes both fields into ShaderComputeStageInfo (request serialization version 37, cache key and dispatch key included). A VALU write to a shared VGPR also updates the partner lane's copy through a new HalfSwapU32 exchange, the high half winning when both halves write, as a wave64 VALU instruction issues its low half first (ISA 4.2.1 Table 9, 6.2.4). A non-VALU write, such as ACO's ds_bpermute_b32 into a shared VGPR, is modeled while an immediate EXEC write earlier in the block has left one half of EXEC empty; with both halves possibly active the ISA gives no order and it throws, as do a VALU instruction that reads the shared VGPR it writes, v_writelane_b32, relative VGPR writes in such a program, and a draw whose SPI_SHADER_PGM_RSRC2.SHARED_VGPR_CNT is set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ion version 50 main now writes request serialization version 49 (37 stencil operation value export, 38 mesh barycentric flag, 39 merged hull flag, 41 DenormPreserve 32, 42 sample order, 43 point list, 44 fused exact arithmetic, 45 pixel function LDS buffer, 46 target merge, 47 two-sample alpha-to-coverage dither, 48 fragment barycentric builtins, 49 depth export among them), so a compute request of version 49 or older written by main would be read with the two shared VGPR words it does not have. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JKJicKPXaPdKfXumezyTiE
a3fe509 to
48fb6f0
Compare
|
Rebased onto
AI-assisted: yes (Claude Code); results above were run on this machine. |
✅ NVIDIA GPU test: passedPR head
Unstable on main (1, 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)Validated but not compared (1, not in the main baseline's validation pass)Validation IDs already present on main (3 IDs in 392 tests)Skipped (48)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. |
What
On GFX10, a wave64 program can declare wave-shared VGPRs (RDNA2 ISA 3.6.5):
COMPUTE_PGM_RSRC3.SHARED_VGPR_CNTblocks of eight VGPRs placed after the(COMPUTE_PGM_RSRC1.VGPRS + 1) * 4private ones. Lane N and lane N + 32 share one copy of each.emit_permlane64_shared_vgpr) and to emulate a wave-wideds_bpermute_b32(emit_bpermute_shared_vgpr).-fa on -ctk q8_0 -ctv q8_0, and slightly wrong text in other configurations.Changes:
RSRC1.VGPRSandRSRC3.SHARED_VGPR_CNTfor compute dispatches intoShaderComputeStageInfo. Both are part of the cache key, the dispatch key (0x228) and request serialization (version 50).HalfSwapU32IR op.ds_bpermute_b32, is modeled while an immediates_mov_b32,s_mov_b64ors_bfm_b64earlier in the block has left one half of EXEC empty, as ACO does. With both halves possibly active the ISA gives no order, so it throws.v_writelane_b32, relative VGPR writes in such a program, wave32 programs with shared VGPRs, and host subgroups narrower than 32 lanes.SHARED_VGPR_CNTinSPI_SHADER_PGM_RSRC2for graphics stages. Graphics stages are not modeled, so a draw whose program sets it throws. No graphics program set it in Dusklight, flycast, Porpoise, PSSwanStation, PS5LM, Prospero Win, ProsperoAI or ProsperoLight.docs/dev/debt/shader.md.Sources:
VGPR_SHARED_SIZEin units of 8).be738ab0ea44cbddee6f0dcbd3cd9d16d98118ed:src/amd/compiler/aco_lower_to_hw_instr.cpp:emit_permlane64_shared_vgpr,emit_bpermute_shared_vgprandcopy_constant_sgpr.src/amd/vulkan/radv_shader.c:S_00B848_VGPRS((num_vgprs - 1) / 4)for wave64,S_00B8A0_SHARED_VGPR_CNT(num_shared_vgprs / 8), and theRSRC2SHARED_VGPR_CNTof the graphics stages.src/amd/registers/gfx10.json: field positions.Tested
Windows 11, WinLibs GCC 15.2, full build:
ctest -R "agc_|shader|recompiler|guest_|request_serializer" -E video_out(GPU tests on): 1163/1163 passed.ctest -R "^(libkernel|kernel|nid|patched_libraries|file_streams|libc)" -E "agc_|video_out": 12/12 passed.After the rebase onto
main3a40945 (serializer version 50): clean full Windows build.request_serializer,agc_shader_memory,agc_driver_wave_shared_vgprsand the wider serializer, dispatch, subgroup, masked-select, stencil and shader-cache set pass 40/40 on NVIDIA and on AMD.ctest -R ^agc_on NVIDIA: 1122/1122.Linux (WSL Ubuntu 24.04, GCC 13.3): libSceAgcDriver plus the recompiler, serializer and AGC driver tests pass.
agc_driver_wave_shared_vgprsskips on llvmpipe (subgroup size 8).New tests:
agc_driver_wave_shared_vgprsruns ACO's half swap, an exec-masked low-half write and ACO's wave64 bpermute sequence word for word, and checks the rejections.main's sources it fails withlane 0 read 0x00000000, expected lane 32's 0x00201020, or withlane 6 read 0x00000000, expected lane 33's 0x00211021for the bpermute part only.agc_driver_prepared_unregistered_graphicschecks that a graphics stage withSHARED_VGPR_CNTis rejected.agc_driver_graphicscovers decodingRSRC1/RSRC3and serialization.request_serializerreplays a version 49 compute request, which has no wave-shared VGPR words. If the reader reads those words from version 37 as before, it fails withRequestSerializer: truncated data.PS5LM Plus 0.8.4 (PPSA99581) with Qwen2.5-0.5B-Instruct q4_k_m (Apache-2.0), prompt "The capital of France is", 16 tokens, temperature 0. The title is fixed on NVIDIA (RTX 5070 Ti):
main,-fa on -ctk q8_0 -ctv q8_0:orntymology. In the 19th-arthury.10(3/3).Paris. It is the largest city in Europe and the second largest in the world(3/3), the CPU backend's exact answer.-fa onwith f16 K/V, and-fa off, now give the same answer. Before, they gaveParis. It is the largest city in the world by population…andParis. It is located in the south of France….On the AMD Radeon iGPU the title is still garbage in every configuration,
-fa offincluded, both before and after this change; that is a separate bug. Only the replay of the captured flash-attention dispatch is correct there: its max relative error against a numpy reference went from 0.99 to 0.031.An independent review checked:
A regression run of Dusklight, flycast, Porpoise, PSSwanStation, PS5LM, Prospero Win, ProsperoAI and ProsperoLight (60 s each) showed no new errors.
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)🤖 Generated with Claude Code