Skip to content

feat(shader): store D16 data to SINT storage images - #5899

Merged
adriadam10 merged 2 commits into
boykopovar:mainfrom
ps54all:feat/shader-image-store-d16-sint
Oct 16, 2026
Merged

adriadam10 merged 2 commits into
boykopovar:mainfrom
ps54all:feat/shader-image-store-d16-sint

Conversation

@ps54all

@ps54all ps54all commented Oct 14, 2026 •

Copy link
Copy Markdown

What

image_store with D16 data into a storage image of a SINT format threw stores 16-bit data to an image of a SINT format. God of War Ragnarök (PPSA08330) hits it in compute shader 0x1052a17900 right after its MFSR setup (#5796).

The throw was in two places:

  • the descriptor decode (ResourceMaterializer);
  • the SPIR-V store (StoreTexel).

RuntimeImageModes also left the SINT conversions out for images with 16-bit data.

  • Each 16-bit half is now converted by EmitD16StoreComponent, the function the D16 format buffer stores already use: sign-extend, then saturate to the component width the same way as for 32-bit data.
  • feat(shader): implement the D16 format buffer stores #331 measured that rule for D16 buffer stores on RDNA2. Image and buffer stores share the texture unit's format conversion, but nobody has measured the image form. The SINT storage image entry in docs/dev/debt/shader.md now says so.
  • The UINT and float D16 image paths are unchanged.

Tested

  • Windows 11, RTX 3080 (NVIDIA 610.74), MinGW-w64 GCC 15.2.0 (winlibs), Release with BUILD_TESTING=ON, on main 5a6ad0e.
  • agc_driver_image_store_sint:
    • Now stores D16 data with DMASK 0xf and 0x5 to all eight SINT formats it covers, and once after an image_atomic_add on 32_SINT. It checks every texel against the model "16-bit half → sign-extend → saturate". The input edge values include 0x7fff, 0x8000, 0xffff and 0x0000.
    • Passes with the change.
    • Without the change it fails with the refusal.
    • When the image emitter zero-extends instead of sign-extending, it fails (16_SINT texel (2, 0) component 0 is 0x00007fff, expected 0x0000ffff).
  • ctest -R "agc|shader|recompiler": 536/536 pass.
  • tools/check_conventions.py --base origin/main and tools/check_test_registration.py pass.
  • PPSA08330 (relinked, on a local branch with feat(psml): implement the MFSR shared-resource and 800 context setup #5796 and other open PRs):
    • Before: it stops at about 30 s on the refusal in compute shader 0x1052a17900.
    • With this change it gets past that dispatch, opens its window and runs to about 60 s.
    • The next stop is unrelated to this change: compute shader 0x10527dd300: sampling, gathering or querying LOD of a converted unorm image is not implemented (the 11_11_10 UNORM sampling gap in the debt list).
  • Not tested: Linux (CI), AMD hardware, a console.

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; no silent stubs added
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source (none)
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: PPSA08330 past this stop (with feat(psml): implement the MFSR shared-resource and 800 context setup #5796 for its MFSR setup)
  • GPUs tested: NVIDIA RTX 3080 (610.74)
  • AI-assisted: yes. Claude Code wrote the change and the test and ran them; the sign-extension rule is the one feat(shader): implement the D16 format buffer stores #331 measured for D16 buffer stores.

🤖 Generated with Claude Code

@github-actions github-actions Bot added shader core/shader docs agc libSceAgc* needs rebase Conflicts with main, rebase needed labels Oct 14, 2026
@ps54all
ps54all force-pushed the feat/shader-image-store-d16-sint branch from 51768ef to 8c11937 Compare October 14, 2026 16:25
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 14, 2026

@jotitadev jotitadev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Run on an Apple GPU, since the PR was tested only on an RTX 3080.

GPUs tested: Apple M2 Max, MoltenVK 1.4.2 (Vulkan 1.4.357), macOS 27.2, x86-64 under Rosetta

main does not run the GPU tests on macOS yet, so I tested on macos-port at b7eb6b6b (mugurc/AnyPS5) plus the ImportWatched change from #1, with this PR's own diff (merge-base..head 8c1193744) applied. Not main. Release build, LunarG Vulkan SDK 1.4.363.0. Suite: ctest -R "^agc" -E "agc_display_buffer_dcc|agc_driver_flip|agc_driver_lds_atomics64" -j1 --timeout 30. Without the PR, 13 tests fail on this setup (7 with the #2704 build), all known macOS failures listed in #1. One hunk in ResourceMaterializer.cpp conflicts trivially: the branch's throw has another condition. I resolved it to the PR's side, which removes the throw.

  • agc_driver_image_store_sint passes. That covers the D16 stores with DMASK 0xf and 0x5 to all eight SINT formats, and the store after image_atomic_add.
  • With only the new test on the unchanged branch, it fails with stores 16-bit data to an image of a SINT format, the refusal in the description.
  • Suite: the same 13 failures with and without the PR. None new, none fixed.

Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.

@ps54all

ps54all commented Oct 14, 2026

Copy link
Copy Markdown
Author

Thanks for the M2 Max / MoltenVK run. agc_driver_image_store_sint passing there (DMASK 0xf and 0x5, all eight SINT formats, the store after image_atomic_add) and failing with the refusal message on the unchanged branch is exactly what the test is meant to show.

This was referenced Oct 15, 2026
@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

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 907 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
core/shader/recompiler/SpirvBackend/src/SpirvImageEmitter.cpp
PR 1341 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
PR 1896 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
PR 5371 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
core/shader/recompiler/SpirvBackend/src/SpirvImageEmitter.cpp
docs/dev/debt/shader.md
PR 5426 docs/dev/debt/shader.md
PR 5500 docs/dev/debt/shader.md
PR 5867 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
PR 6054 docs/dev/debt/shader.md
PR 6222 docs/dev/debt/shader.md
PR 6235 docs/dev/debt/shader.md
PR 6312 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
PR 6412 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
docs/dev/debt/shader.md
PR 6612 docs/dev/debt/shader.md

@coltken95

Copy link
Copy Markdown

Tested on Linux x86-64 (Release build, lavapipe / llvmpipe LLVM 20.1.2), with 8c11937 merged into main 44df216:

  • With the PR: agc_driver_image_store_sint passes.
  • With main's sources: it fails with ShaderRecompiler::Recompile: stores 16-bit data to an image of a SINT format.

AI-assisted: yes (Claude Code)

@adriadam10 adriadam10 self-assigned this Oct 16, 2026
ps54all and others added 2 commits October 16, 2026 13:18
image_store with D16 data to a storage image of a SINT format threw
"stores 16-bit data to an image of a SINT format", both when the
descriptor is decoded and in the SPIR-V image emitter, and the runtime
image modes left out the SINT conversions for such images. God of War
Ragnarok (PPSA08330) stops on it in compute shader 0x1052a17900 right
after its MFSR setup.

Each 16-bit half is now converted like the D16 format buffer stores
(EmitD16StoreComponent): sign-extended, then saturated to the
component width as for 32-bit data. The buffer form was measured on
RDNA2 in boykopovar#331; the image form is not measured (TechnicalDebt).

agc_driver_image_store_sint now stores D16 data with DMASK 0xf and 0x5
to all eight SINT formats and after an image_atomic_add on 32_SINT,
instead of expecting the refusal.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@adriadam10

Copy link
Copy Markdown
Collaborator

Measured image_store d16 to all eight SINT formats on RDNA2 (gfx1035) with DMASK 0xf, 0x7, 0x5, 0x3, 0x2, 0xa and 0x1 over edge halves (1792 rows): every row matches the sign-extend-then-saturate model, and 765 of them would differ with zero-extension. Build and the AgcDriver/Agc tests are green on RADV; agc_driver_image_store_sint fails without the change.

Added on top: the docs/dev/debt/shader.md entry now says the D16 SINT image store rule is measured instead of "not measured". Merging.

@adriadam10
adriadam10 force-pushed the feat/shader-image-store-d16-sint branch from 8c11937 to d8c2b71 Compare October 16, 2026 11:25
@adriadam10
adriadam10 merged commit bba8b29 into boykopovar:main Oct 16, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* docs shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants