Skip to content

fix(shader): add the ordered count of a first active lane in the upper wave64 half - #6258

Merged
adriadam10 merged 2 commits into
boykopovar:mainfrom
erm14254:fix/shader-ordered-count-wave64
Oct 18, 2026
Merged

adriadam10 merged 2 commits into
boykopovar:mainfrom
erm14254:fix/shader-ordered-count-wave64

Conversation

@erm14254

Copy link
Copy Markdown

What

ds_ordered_count is emitted as a device-scope atomic add of one active lane's value. The amount was read with a 32-wide shuffle at first & 31, so when the first active lane of a wave64 wave sits in the upper half, the lower half's lane was added instead. The amount now comes from the wave64 Shuffle helper, which reads the first active lane in either half.

Tests: agc_driver_gds_append_consume gains ordered runs whose first active lane is in the upper half, with M0 set, and checks that inactive lanes stay untouched and that waves see sorted bases; agc_wave_control_decode recompiles the ordered code for 32-wide hosts; agc_driver_pm4 covers the ordered-append modifiers on indirect dispatches (0x49 accepted, 0x51 refused).

Tested

Windows 11 (10.0.22631), RTX PRO 6000 Blackwell, NVIDIA 597.16: agc_driver_pm4, agc_wave_control_decode and agc_driver_gds_append_consume pass. Linux, lavapipe: agc_driver_pm4 and agc_wave_control_decode pass, the GDS test skips without a device. Code Violet (PPSA26528) gets past its first ordered-append dispatch with this and reaches its title screen together with the other pull requests listed in its bring-up thread.

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 under Silent stubs in their library's technical debt file
  • 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: PPSA26528 Code Violet (PPSA26528) #6256
  • GPUs tested: NVIDIA RTX PRO 6000 Blackwell Workstation Edition, driver 597.16, Windows 11; lavapipe on Linux
  • AI-assisted: yes

…r wave64 half

The ordered count's amount was shuffled from the first half's value of
the first active lane's host lane, so a wave64 program whose active lanes
were all in lanes 32 to 63 added the inactive lane's value from the lower
half. Read the amount through the context shuffle, which selects the half
the first active guest lane lives in, as v_readfirstlane_b32 does.

The GDS test now masks the ordered count to the upper part of each wave,
including a wave64 whose first active lane is lane 40, prefills the
result register to check that inactive lanes stay untouched, and keeps
the M0 cases of the counter addressing.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019hfwvntiwabeqdZ6avJssk
DISPATCH_INDIRECT accepts ORDERED_APPEND_ENBL like DISPATCH_DIRECT and
still refuses ORDERED_APPEND_MODE.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019hfwvntiwabeqdZ6avJssk
@github-actions github-actions Bot added shader core/shader agc libSceAgc* labels Oct 15, 2026
This was referenced Oct 15, 2026
@erm14254

Copy link
Copy Markdown
Author

Manual overlap review for the list above, against each pull request's current diff:

This PR changes the ordered amount read earlier in AppendConsume, separated from that hunk by unchanged lines, so the two merge in either order. No overlap, no Depends on.

@erm14254

Copy link
Copy Markdown
Author

Manual overlap review for #6901: its changes to these four files are line endings only. Its copies of Pm4.cpp, WaveControlDecode.cpp, GdsAppendConsume.cpp and SpirvMemoryInstructions.cpp are identical to its merge base once the carriage returns it adds are stripped; it converts every file in the repository the same way (+515,598 −513,181). No overlap, no Depends on.

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
PR 7429 core/libs/prx/libSceAgcDriver/tests/WaveControlDecode.cpp L106
core/libs/prx/libSceAgcDriver/tests/execution/GdsAppendConsume.cpp L29-30, L52-53, L206, L208

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.

Pull request Shared files not compared
PR 5021 core/shader/recompiler/SpirvBackend/src/SpirvMemory/SpirvMemoryInstructions.cpp
PR 6185 core/libs/prx/libSceAgcDriver/tests/Pm4.cpp
PR 6186 core/libs/prx/libSceAgcDriver/tests/Pm4.cpp
PR 6549 core/libs/prx/libSceAgcDriver/tests/Pm4.cpp
PR 7008 core/libs/prx/libSceAgcDriver/tests/Pm4.cpp
PR 7332 core/shader/recompiler/SpirvBackend/src/SpirvMemory/SpirvMemoryInstructions.cpp

@erm14254

Copy link
Copy Markdown
Author

Manual overlap review for #5021: in SpirvMemoryInstructions.cpp it now carries the same single hunk as #3891 in the first review above, the result shuffle at the end of AppendConsume (EmitSubgroupShuffle to EmitLaneShuffle), identical line for line. A three-way merge of that file with this PR is clean and keeps both changes. No overlap, no Depends on.

@savvaniss

Copy link
Copy Markdown
Contributor

Intel and AMD Windows data: this fixes the upper-half case on an Intel UHD 770. On an RX 9070 XT the case passes with and without it.

Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release, every execution test built, ANYPS5_NO_SHADER_CACHE=1, serial CTest. Intel UHD Graphics 770 (8086:a780), driver 32.0.101.7088. AMD Radeon RX 9070 XT (1002:7550), driver 32.0.31041.1004, with VkCache emptied first. "Without" means this PR's test files on main c9e6c45de, with none of its other changes.

UHD 770 RX 9070 XT
This head 411ce81a6 merged into main ca82d7d88: agc_driver_gds_append_consume, agc_wave_control_decode and the 13 agc_driver_pm4* tests, ×3 all pass all pass
Without: agc_driver_gds_append_consume, ×3 fails: wave64 64 threads, 40 active ds_ordered_count with M0 7 from lane 40 of each wave: GDS counter 0 is 1011, expected 1043 passes

The RX 9070 XT runs 64-wide subgroups (the UHD 770 runs 32-wide), and there the new case passes without the fix too. So the UHD 770 is the device here that exercises the fixed path.

Full CTest (--parallel 4) on main c9e6c45de with seven open GPU PRs merged (#6258, #6657, #6324, #6246, #6174, #6360, #6598): UHD 770 1381 of 1389, RX 9070 XT 1383 of 1389. main alone passes 1378 of 1385 and 1381 of 1385. The merge adds only two failures: agc_buffer_pool (#6174, both GPUs) and agc_driver_indirect_dispatch_limit (#6360, RX 9070 XT).

AI-assisted: yes (Claude Code)

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

Labels

agc libSceAgc* shader core/shader unblocks title Names a title ID on the Unblocks line

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants