Skip to content

fix(agc): order raw shader reads after queued code writes - #6326

Open
JJenkx wants to merge 1 commit into
boykopovar:mainfrom
JJenkx:fix/queued-raw-shader-reads
Open

JJenkx wants to merge 1 commit into
boykopovar:mainfrom
JJenkx:fix/queued-raw-shader-reads

Conversation

@JJenkx

@JJenkx JJenkx commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

What

A raw shader read behind a queued write to the same code can return the old program. Queue-0 draw commits now run through DrawPipeline, but ReadRawShader only flushes work already recorded by the GPU recorder. A pending pipeline commit is not yet visible to that flush. Both a cached-code comparison and a first-time code read can therefore get ahead of the write.

Drain overlapping queued writes before comparing a cached raw snapshot and before reading each new code span. The drain runs outside the raw-shader cache mutex. Reads outside the queue-0 pipeline, and reads with no overlapping pending write, retain the existing path. No shader instruction semantics, registration policy, or cache format changes.

The CPU-only agc_driver_queued_raw_shader regression exercises cached and fresh reads through the real pipeline committer and guest-memory write path. The queued writer waits until the read returns or its bounded delay expires; the read must observe the replacement program. The cases also run separately as agc_driver_queued_raw_shader_tests cached and ... fresh. No PS5 assets or GPU are required by this regression.

This is independent of #6188 and needs no commits from it. #5751 changes registration/range bookkeeping in the same source file; this PR changes ordering of code reads and does not replace that work.

Tested

Rebased as one topic commit 3018bc659f4a126b4b379079456df6004e2b6aed on current main 7b12450b8ffe58cee26bee6cbf63740c6188c6b2. Resolved the additive CMake conflict by keeping main’s draw-pipeline trace settings and disabled-tracing case alongside the queued raw shader settings. Production and regression-test code are unchanged by the rebase. Linux x86-64 GCC Release, SPIRV-Tools ON at the unchanged official pin 7f2d9ee926f98fc77a3ed1e1e0f113b8c9c49458: full runtime libraries and seven focused test executables build. All 11 selected CTest cases pass on both Radeon RX 6650 XT / RADV (1.44 seconds) and lavapipe (1.62 seconds), zero skips: queued raw shader; draw-cache reuse in default, GPL-off, verification and inline modes; draw-write ranges; pipelined DMA; predicated label order; predication read; and main’s draw-pipeline tracing enabled/disabled cases. All seven execution fixtures were explicitly selected. Conventions and whitespace checks pass. The overlap check reports this PR itself; the 54 unavailable shared-file comparisons were inspected from fetched peer branches, with no added queued-raw-shader fixture or changes to this read-ordering path found. The complete suite and Windows were not rerun for this rebase.

Earlier local validation on main 341613ab62b979e429daa1be5ac2fe524a737df6, public head 6cd8b7240: Linux x86-64 GCC Release, renderer SPIRV-Tools enabled: full runtime libraries and all selected executables build. All 10 selected CTest cases pass without skips on both Radeon RX 6650 XT / RADV Mesa 26.2.4 and lavapipe (6.47 s / 6.58 s). Besides the new CPU regression, these cover draw-cache reuse with GPL on/off, validation and inline modes, draw-write ranges, pipelined DMA/fills and predication. Conventions and whitespace pass.

Earlier negative controls replace only ShaderRegistry.cpp with main's version. Both selectors exit 1 with cached raw shader read ignored a queued write to its code / fresh raw shader read ignored a queued write to its code. The executable SHA-256 stays 4f87301733885662f3e6422eb053b95aab8d53372269a63dcb1c18a6f4189bbd through these controls and the restored-source build. The fix is restored and the full libraries rebuilt before the final passing tests.

That earlier local validation tree differs from its public head only by a private SPIRV-Tools teardown-repair gitlink, excluded from this PR. Those earlier local results do not independently validate the official dependency pin; the refreshed focused checks above do. Contributor-reported official-pin and full-suite results are recorded below; Independent Windows validation of the refreshed exact head is reported below. No title unblock or FPS improvement is claimed.

Contributor-reported validation by Dextroid17: head 6cd8b7240 cleanly merged onto main 84b0d2f24, CachyOS Linux 7.2.9, GCC 16.2.1, Release, lavapipe, ANYPS5_REQUIRE_VULKAN=1, ctest -j16.

  • With SPIRV-Tools ON and the unchanged official pin 7f2d9ee92, cached and fresh selectors each pass 25/25 with this fix and fail 25/25 with only ShaderRegistry.cpp restored from main; restoring the fix and rebuilding passes another 25/25 each.
  • The ON full suite reports 1368/1369 in each of three runs, with 62 tests skipped on lavapipe. agc_compute_pipeline_replay fails with replay did not export a pipeline cache header; the contributor reproduces that failure with main's ShaderRegistry.cpp as well. These are not all-green full-suite results.
  • With SPIRV-Tools OFF, the full suite reports 1361/1361 in each of three runs. The queued raw shader regression passes 50/50 with the fix and fails 50/50 with main's ShaderRegistry.cpp. Conventions pass on the merged tree.

These results were reported by the contributor, whose testing was AI-assisted with Claude Code; they were not reproduced locally. That earlier contributor report did not include Windows testing.

Independent Windows validation reported by Amoenus covers exact head 3018bc659f4a126b4b379079456df6004e2b6aed, Windows 11 Enterprise 26200, WinLibs GCC 15.2r7 Release, SPIRV-Tools ON at the official pin, and clean submodules. Full runtime libraries plus seven selected execution fixtures build. The queued regression passes 3/3, and cached/fresh selectors each pass 3/3; replacing only ShaderRegistry.cpp with its parent makes both selectors fail 3/3, and restoring the fix restores all nine passes. The regression executable hash stays unchanged. Neighboring coverage reports 30 GPU executions on each source configuration (15 NVIDIA RTX 5080 and 15 AMD Radeon Graphics) with required Vulkan, disabled caches, and Khronos validation: zero errors/VUIDs and matching 60 shader-output warnings. Additional selected CPU cases pass. Existing analyzer findings reproduce on the parent. These are contributor-reported focused results, not locally reproduced Windows tests, an unmodified current-main comparison, full-suite results, or a title/performance claim. Testing was AI-assisted.

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 (none added)
  • Generic behaviour, not specific to one title
  • Depends on: none (perf(agc): pipeline queue-0 draw commits behind the packet worker #1201/perf(agc): pipelined draws on by default #1202 are merged)
  • Fixes: none
  • Unblocks: none
  • GPUs tested: AMD Radeon RX 6650 XT / RADV Mesa 26.2.4, Linux; lavapipe Mesa 26.2.4, Linux (the queued raw shader regression itself is CPU-only)
  • AI-assisted: yes

@github-actions

github-actions Bot commented Oct 15, 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 7644 core/libs/prx/libSceAgcDriver/CMakeLists.txt L2073

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 1368 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 1395 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 2434 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 4891 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 5021 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 5115 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp
PR 5371 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 5426 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 5431 core/libs/prx/libSceAgcDriver/CMakeLists.txt
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp
PR 5499 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 5925 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6044 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6146 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6185 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6186 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6187 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6188 core/libs/prx/libSceAgcDriver/CMakeLists.txt
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp
PR 6192 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6209 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6280 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6300 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6412 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6481 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6501 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6611 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6682 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6683 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6718 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6747 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6759 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6788 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6808 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6820 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6867 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp
PR 6868 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6937 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 6946 core/libs/prx/libSceAgcDriver/CMakeLists.txt
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp
PR 6947 core/libs/prx/libSceAgcDriver/CMakeLists.txt
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp
PR 6977 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp
PR 7048 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 7085 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 7201 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 7223 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 7332 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 7395 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 7446 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 7457 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 7464 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 7505 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 7514 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 7604 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 7615 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 7642 core/libs/prx/libSceAgcDriver/CMakeLists.txt
core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp
PR 7677 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 7849 core/libs/prx/libSceAgcDriver/CMakeLists.txt
PR 7933 core/libs/prx/libSceAgcDriver/CMakeLists.txt

This was referenced Oct 15, 2026
@Dextroid17

Copy link
Copy Markdown
Contributor

I ran this on Linux with the official SPIRV-Tools pin and the full test suite, the two things your Tested section lists as not covered.

Setup: head 6cd8b7240 merged onto main 84b0d2f24 (clean merge; the branch is 1,020 commits behind), CachyOS Linux 7.2.9, GCC 16.2.1, Release, -DANYPS5_ENABLE_SPIRV_TOOLS=ON as in build.yml, with 3rdparty/SPIRV-Tools at the pinned 7f2d9ee92 and no local changes. Tests ran on lavapipe with ANYPS5_REQUIRE_VULKAN=1, ctest -j16.

SPIRV-Tools ON, official pin                          with this PR   ShaderRegistry.cpp from main
agc_driver_queued_raw_shader_tests cached (25 runs)   25/25 pass     0/25 ("cached raw shader read ignored a queued write to its code")
agc_driver_queued_raw_shader_tests fresh  (25 runs)   25/25 pass     0/25 ("fresh raw shader read ignored a queued write to its code")
after restoring the file and rebuilding               25/25 each
full ctest, 3 runs                                    1368/1369 each (the one failure is local, see below), 62 skipped on lavapipe

The one failure, agc_compute_pipeline_replay (replay did not export a pipeline cache header), also fails on my machine with ShaderRegistry.cpp from main, and it passes on CI's main build, so it is something local to my setup and not this change.

With SPIRV-Tools OFF (the CMake default), full ctest passed 1361/1361 in each of 3 runs, and agc_driver_queued_raw_shader passed 50/50 through ctest with the fix and 0/50 with ShaderRegistry.cpp from main. tools/check_conventions.py passes on the merge.

I did not test Windows.

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

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 18, 2026
@JJenkx
JJenkx force-pushed the fix/queued-raw-shader-reads branch from 6cd8b72 to 563bdee Compare October 18, 2026 22:44
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 18, 2026
@adriadam10 adriadam10 self-assigned this Oct 18, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 18, 2026
@JJenkx
JJenkx force-pushed the fix/queued-raw-shader-reads branch from 563bdee to 1fd833c Compare October 18, 2026 23:38
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 18, 2026
@JJenkx
JJenkx force-pushed the fix/queued-raw-shader-reads branch from 1fd833c to 3018bc6 Compare October 19, 2026 00:59
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 19, 2026
@Amoenus

Amoenus commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

@JJenkx independently validated exact head 3018bc659f4a126b4b379079456df6004e2b6aed on Windows 11 Enterprise 26200, WinLibs GCC15.2r7 Release, SPIRV-Tools ON at official pin 7f2d9ee926f98fc77a3ed1e1e0f113b8c9c49458. All submodule worktrees matched their gitlinks. Built full runtime libraries plus the seven explicitly selected execution fixtures; no local changes in the final tree.

Regression and failure control

  • New agc_driver_queued_raw_shader CTest: 3/3 pass. Direct cached and fresh selectors: 3/3 pass each (nine CPU-only passing processes total).
  • Replace only ShaderRegistry.cpp with exact parent 7b12450b8ffe58cee26bee6cbf63740c6188c6b2, rebuild: cached 3/3 fail and fresh 3/3 fail, exit1 with their respective “raw shader read ignored a queued write to its code” errors.
  • Restore the original PR source byte-for-byte and rebuild libraries: all nine CPU-only processes pass again.
  • Regression executable SHA256 stays 7958598ad46f132da09d3004cb00f62d99a52016a3a4d7f83eb4f21a53c2a76f throughout.

Neighboring coverage and diagnostics

On both the candidate and parent-source control, repeated the same five GPU cases three times per physical device: draw-cache reuse in default/GPL-off/verification/inline modes, and pipelined predication read. 30 verified GPU executions on each source configuration: 15 NVIDIA RTX5080 (610.62 /32.0.16.1062), 15 AMD Radeon(TM) Graphics (32.0.21036.18). Loader output confirms the actual selected devices.

Five other cases ran three times each as 15 CPU-only passes per configuration: draw-write ranges, pipelined DMA, predicated label order, and committer tracing enabled/disabled. They created no Vulkan device and are excluded from GPU counts. Thus the candidate has 30 GPU +24 CPU-only passes, the parent control 30 GPU +15 CPU-only passes +6 expected regression failures, and the restored run adds 9 CPU-only passes.

GPU runs used verbose CTest, 30s timeout, required Vulkan, disabled shader cache/pipeline journal, and loaded Khronos validation1.4.363.0. The queued regression uses APS5_PIPELINED_DRAWS=8; other cases retain their registered mode settings. Zero validation errors or VUIDs on both configurations. Draw-cache cases emit 60 shader-output-not-consumed warnings on each configuration, with matching IDs/counts in every common case (Undefined-Value-ShaderOutputNotConsumed[-DynamicRendering]). This is no new diagnostics, not a zero-diagnostic suite.

Conventions passes. Cppcheck/clang-tidy analysis remains recorded as findings: one Cppcheck finding and three Clang advisories in existing registry/timer code reproduce on the parent; the new fixture has no selected Clang findings. The unchanged rethrow helper's Cppcheck missingReturn was adjudicated as a false positive: std::rethrow_exception is noreturn in C++20, and its catch also throws. No production cleanup or analyzer suppression was added.

Scope: exact PR head plus a one-file parent fault control, not an unmodified current-main build, current-main merge composition, full default build, full suite, title or performance result. Tested2026-10-20.

AI-assisted: yes (Codex); author responsible.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants