Skip to content

fix(shader): decode GFX10 scratch offsets as one dword - #5426

Open
Prophet069 wants to merge 3 commits into
boykopovar:mainfrom
Prophet069:codex/review-scratch-offsets-20261013
Open

Prophet069 wants to merge 3 commits into
boykopovar:mainfrom
Prophet069:codex/review-scratch-offsets-20261013

Conversation

@Prophet069

Copy link
Copy Markdown
Contributor

What

GFX10 scratch SV addresses contain one32-bit VGPR offset, not the VGPR pair used for ordinary FLAT addressing. VGPR255 was therefore incorrectly rejected. Decode SV/SS/ST address components independently and make the scratch IR consume the offset components; retain the FLAT/global VGPR-pair and unsupported-scalar checks. Source: LLVM FLATInstructions.td for the operand widths and SV/SS/ST pseudos, and GFX10 encoding for null/EXEC_HI sentinels. #1190 changes flat-aperture SPIR-V routing and does not modify these decoder/IR units.

Replaces the obsolete base of closed #2965, which was automatically closed for intake pause. Targeted current GitHub search found no other open PR implementing this exact export/decoder correction. Related #5005 is a scalar-aperture change, not the one-dword scratch decoder fix.

Tested

Windows 10 Pro 22H2 x64 (build 19045), MinGW GCC 15.2.0: fresh production decoder, IR, translation and control-flow sources compiled on this candidate; all 14 valid/invalid scratch/FLAT decoder and IR cases passed (0.06 s). The standalone test uses the existing Windows .eh_frame to .ehfram configuration and the freshly built libc import library. No Vulkan or PS5 hardware scratch-address result is claimed. The instruction dc304000 027d00ff was originally captured from Death Stranding 2: On the Beach, PS5 title ID PPSA02015, in an earlier experimental integration. This is not a claim that this isolated PR renders or boots the game.

Based on main dacd78525e537a779682032c0ace7f7af120af05; candidate cc1dc3f1c6f88cd03665f933e0b7590e5717b0fe. No dumps, runtime packages, investigation reports or images are included. No speed improvement is claimed.

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 success stub added
  • No notes, investigation .md files or images added to the repository
  • New third-party code: none
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: PPSA02015 Death Stranding 2: On the Beach (PPSA02015) #5424 (captured decoder stop; no game rendering or hardware scratch-address claim)
  • GPUs tested: not tested on refreshed candidate; prior decoder/IR tests are CPU tests (Windows)
  • AI-assisted: yes; prepared with Codex; the submitting author is responsible for every line

@github-actions github-actions Bot added build shader core/shader docs agc libSceAgc* labels Oct 13, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
RINNECODER added a commit to RINNECODER/AnyPS5 that referenced this pull request Oct 14, 2026
…t) (#481)

* fix(recompiler): give vertex-path shaders the NGG group info

Ported from upstream PR boykopovar#5081

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

* fix(shader): put the compute wave index in TG_SIZE[11:6]

Ported from upstream PR boykopovar#5214

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

* fix(shader): take the 32_AR export alpha from channel 1 as GFX10 does

Ported from upstream PR boykopovar#5110

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

* fix(shader): order buffer and image memory at s_barrier

Ported from upstream PR boykopovar#5117

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

* fix(shader): decode GFX10 scratch offsets as one dword

Ported from upstream PR boykopovar#5426

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

* fix(shader): accept MIMG sampler tuples ending in VCC

Ported from upstream PR boykopovar#4996

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

* fix(recompiler): name the attribute of an out-of-range interpolation reference

Ported from upstream PR boykopovar#5450

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

* fix(shader): decode the high-half result of v_interp_p2_f16

Ported from upstream PR boykopovar#5471. The Vop3InterpDecode unit-test hunk is not ported (that test file is not on main); the execution test InterpolationF16 carries the coverage.

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

* fix(shader): clear the bits of inactive lanes in VALU carry-out and v_div_scale masks

Ported from upstream PR boykopovar#5162

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

* fix(shader): apply the f32 output modifier and denormal flush to v_cvt_f32_f64

Ported from upstream PR boykopovar#5159

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

* fix(shader): update per-invocation LDS without atomics

Ported from upstream PR boykopovar#5160. Conflicts resolved against main: kept main's barrier condition for non-LDS shared atomics; the unit-test hunk for SRT read patching (a different upstream change, not on main) is dropped.

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

* fix(shader): flush ds_add_f32 denormals and NaNs as the hardware does

Ported from upstream PR boykopovar#5492

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

* fix(shader): run DS src2 lanes one at a time in lane order

Ported from upstream PR boykopovar#5498

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

* fix(agc): compare NOT_EQUAL unordered, reject TRUNC_COORD point sampling and round the array layer half to even in color compare emulation

Ported from upstream PR boykopovar#907 (follow-up to its predecessor). Hand-applied to main's reworked emitter: NOT_EQUAL uses OpFUnordNotEqual, the array layer is clamp(roundEven(z)), and ResourceMaterializer rejects TRUNC_COORD with point filtering. The upstream execution-test rewrite is not ported (it targets a diverged test file).

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

* chore(shader): bump ShaderDiskCache FormatVersion to 35

The ported DS fixes add MemoryInfo::flushDenormalResults (now encoded in the plan) and new IR opcodes, so cached payloads from format 34 must not load.

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

* test(shader): run ported upstream execution tests on Metal through a device shim

The upstream execution tests for the ported fixes compile unchanged against a Metal stand-in for the Vulkan test device (guest buffer descriptors are mirrored into shared MTLBuffers). Covers boykopovar#5162 (VALU carry-out masks), boykopovar#5159 (v_cvt_f32_f64 result modes), boykopovar#5492 (ds_add_f32) and boykopovar#5498 (DS src2 lane order). The LdsSrc2 and LdsSrc2Wide tests use device-memory LDS, which the Metal bridge rejects, and are not registered.

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

* test(shader): run ported decoder and collector unit tests in the Metal build

Registers the upstream tests for boykopovar#4996, boykopovar#5426 and boykopovar#5450 (already on main's tree but not built for Metal) and a high-half decode check for boykopovar#5471.

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

* test(shader): check per-invocation LDS updates without atomics on the Metal recompiler

Standalone copy of the boykopovar#5160 verifyFunctionLdsUpdates check.

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

* test(shader): check the TG_SIZE wave index on Metal

Buffer-store variant of the boykopovar#5214 execution test.

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

* test(shader): run the s_barrier memory-order execution test on Metal

Registers boykopovar#5117's upstream test through the device shim.

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

* test(shader): draw a 32_AR export on Metal

Guest-replay draw for boykopovar#5110: alpha comes from export channel 1.

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

* test(shader): draw an NGG vertex shader that reads the group info

Guest-replay draw for boykopovar#5081.

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

* test(shader): reject TRUNC_COORD point sampling in the color compare emulation

Recompile-level check for boykopovar#907.

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

* test(shader): run the src2 lane-order test without the Metal debug layer

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

* docs(shader): port upstream debt entries and add the FormatVersion 35 history line

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DgW4n3qYgi5jAAohEmXRQj

---------

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
@KauaSantos702

Copy link
Copy Markdown
Contributor

NVIDIA run at this head (cc1dc3f1c), adding a vendor to the AMD results in the body: no agc_* test fails.

  • NVIDIA GeForce RTX 5070 (Blackwell), driver 32.0.16.1714, Windows 11
  • WinLibs GCC 15.2.0 r7, Release, BUILD_TESTING=ON
  • ctest -R '^agc_' -j2: 484 tests, 483 passed, 0 failed, 1 skipped. The skipped test is agc_driver_stencil_ref_export, because the device lacks VK_EXT_shader_stencil_export; it is also skipped on main.

@adriadam10 adriadam10 self-assigned this Oct 14, 2026

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Checked the decoder/IR change on a Linux build merged onto current main: agc_flat_scratch passes and fails without the fix, so the logic looks right. Two things before it can merge:

  • The PR conflicts with main in docs/dev/debt/shader.md (TechnicalDebt adjacency) and "Allow edits from maintainers" is off, so we can't rebase it for you. Please rebase on current main, and put the new ### GFX10 scratch address operands entry away from the end of the file, where other PRs append.
  • core/libs/prx/libSceAgcDriver/tests/FlatScratch.cpp is not formatted like the other tests (no spaces around operators, several statements per line, Scratch/Reject helpers on one line); please run clang-format on it and drop the doubled blank line before the new block in libSceAgcDriver/CMakeLists.txt (line 2241-2242).

Not covered: the real-hardware result of a scratch access with a VGPR offset; the TechnicalDebt entry already says so.

@fma-rc

fma-rc commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

A title reaches this: Death Stranding 2 (PPSA02015, 01.015.000) on Linux. It only gets this far with a local fix to the AMM address range (on current main it stops at 1 s on that, PR to follow), and then it registers a shader with

scratch_load_dword v2, v255, off   // dc304000 027d00ff

in its spill code (word 0x553 of a 0xd70-word program, well before s_endpgm; LLVM gfx1030 decodes it the same). Without this PR it stops at 39.5 s with FLAT address register range overflow. With cc1dc3f cherry-picked on top, that throw is gone and it runs to 45 s, where it stops on something unrelated. agc_flat_scratch passes in that build.

@github-actions github-actions Bot added the unblocks title Names a title ID on the Unblocks line label Oct 18, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

@Prophet069 a friendly nudge on this one. The maintainer's 14 Oct review says the logic looks right (agc_flat_scratch passes with the fix and fails without it). What's left is a rebase onto current main, where the only conflict is the adjacent entry in the shader debt doc, and a clang-format pass. Once those are pushed, the change request can be re-reviewed and the PR can go through CI.

@Prophet069

Copy link
Copy Markdown
Contributor Author

@oneandonlydean thank you for the reminder. My ai setup doesnt run currently (swtching to qwen from codex because of slow progression and fast plan usage. Can you rebase it for me? I alllowed edits to be made. Sorry i'm not that knowledgeble with all the github stuff. Just wanted to help without understanding much of it.

Co-authored-by: Prophet069 <291514246+Prophet069@users.noreply.github.com>
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 20, 2026
Format tests/FlatScratch.cpp, drop the doubled blank line before its CMake
block and move the technical debt entry into the Functional list.

Co-authored-by: Prophet069 <291514246+Prophet069@users.noreply.github.com>
@oneandonlydean

Copy link
Copy Markdown
Collaborator

@Prophet069 no problem, and thanks for allowing edits. The branch has main merged in (c1830b3) plus one formatting commit (4c15089): tests/FlatScratch.cpp is clang-formatted, the debt entry has its own place in docs/dev/debt/shader.md, and the extra CMake blank line is gone. Your decoder and translation change is unchanged. CI on that head passed on Linux and Windows, with agc_flat_scratch passing on both. Nothing more is needed from you; we'll take it from here.

@oneandonlydean oneandonlydean self-assigned this Oct 22, 2026
@oneandonlydean
oneandonlydean dismissed adriadam10’s stale review October 22, 2026 19:03

Addressed in 4c15089; CI passed.

This branch has not been deployed

No deployments
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 unblocks title Names a title ID on the Unblocks line

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants