Repository navigation
fix(shader): recognize scalarized constant-stride image tables - #13149
Conversation
There was a problem hiding this comment.
Reviewed — changes requested
The shader repro is author-reported as fixed, but the PR also removes multi-target color merging that is present on its base.
What I ran
Reviewed head 6bf492e804cefd0ef70ab4f4a062aeaa5eeed02d; PR base 5ecef095b61e7a406b279c97896f3a8a6fbd6353. Current main is 422f96b3a92e797c5f69c136c4a394c8da672556; the merge preview is clean (tree 300d4e4445d31e7887adfe6179550a5467fc4b87). Environment: Linux 6.1.0-54-amd64 x86_64. I did not build or run tests locally.
git diff --check upstream/main...upstream/pr-13149
git merge-tree --write-tree upstream/main upstream/pr-13149
python3 tools/check_conventions.py --base upstream/main --head upstream/pr-13149
git diff --unified=4 upstream/main...upstream/pr-13149 -- core/libs/prx/libSceAgcDriver/Graphics/src/State.cpp core/shader/recompiler/Recompiler.hpp core/shader/recompiler/Recompiler.cpp
python3 - <<'PY'
import subprocess
base='5ecef095b61e7a406b279c97896f3a8a6fbd6353'
main='422f96b3a92e797c5f69c136c4a394c8da672556'
head='6bf492e804cefd0ef70ab4f4a062aeaa5eeed02d'
def paths(a,b): return set(subprocess.check_output(['git','diff','--name-only',a,b],text=True).splitlines())
print(sorted(paths(base,main) & paths(base,head)))
PY
gh pr checks 13149 -R boykopovar/AnyPS5Results
| Check | Result |
|---|---|
| Diff check | Passed; no output |
| Current-main merge preview | Clean |
| Conventions | Passed locally and in hosted CI |
| Hosted Windows build, Ubuntu and Windows relinker, relinker ASan | Passed |
| Hosted macOS relinker jobs | Pending |
| Hosted Linux and register-context/write-watch jobs | Skipped |
| Author-reported GPU suite | 63/63 on main and 63/63 on the change, same Windows 10 / RTX 3080 system, with no VUID output; not independently run |
| Author-reported shader repro | On main 08a10d2253, preparation exits 1 with the reported invalid-runtime-value error; on the PR branch it exits 0 with 133,909 SPIR-V words; not independently run |
| Changes on current main since PR base | Four paths; none overlap the PR’s changed paths |
Captured command output
300d4e4445d31e7887adfe6179550a5467fc4b87
All conventions checks passed.
[]
linux skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/41001942789/job/123888190767
register-context skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/41001942789/job/123888191171
write-watch skipping 0 https://github.com/boykopovar/AnyPS5/actions/runs/41001942789/job/123888190820
intake pass 17s https://github.com/boykopovar/AnyPS5/actions/runs/41001942668/job/123888122972
relinker (ubuntu-latest, g++) pass 3m16s https://github.com/boykopovar/AnyPS5/actions/runs/41001942789/job/123888189359
relinker (windows-latest, g++) pass 8m59s https://github.com/boykopovar/AnyPS5/actions/runs/41001942789/job/123888189343
relinker-asan pass 2m46s https://github.com/boykopovar/AnyPS5/actions/runs/41001942789/job/123888189247
windows pass 18m51s https://github.com/boykopovar/AnyPS5/actions/runs/41001942789/job/123888189236
relinker (macos-latest, clang++) pending 0 https://github.com/boykopovar/AnyPS5/actions/runs/41001942789/job/123888189285
relinker (macos-latest, clang++, x86_64) pending 0 https://github.com/boykopovar/AnyPS5/actions/runs/41001942789/job/123888189379
conventions pass 13s https://github.com/boykopovar/AnyPS5/actions/runs/41001942682/job/123888122893
eligibility pass 4s https://github.com/boykopovar/AnyPS5/actions/runs/41001942789/job/123888122754
labels pass 9s https://github.com/boykopovar/AnyPS5/actions/runs/41001942778/job/123888123435
report pass 41s https://github.com/boykopovar/AnyPS5/actions/runs/41001942830/job/123888122166
Negative control
| Assertion | Control and change | Outcome |
|---|---|---|
| Captured PPSA02015 shader preparation | Author reports baseline 08a10d2253 versus the PR branch |
Baseline failed with the cited runtime-value error; PR branch prepared successfully. This is author-provided, not an independently run negative control. |
| Multi-target regression | No local test mutation was run | Not exercised locally; the source change itself rejects a second merged target. |
Findings
Blocking
- Restores a one-target limit and removes merged-target handling. Current
mainsupports multiple partial-write/unclamped-normalized target merges:ExportTargetMergesreturns a vector, and the recompiler materializes and hashes all entries. This PR changes that to a singleTargetMergeand addsRequire(merge.mask == 0, ...), which rejects a draw with a second mergeable target. The PR description does not identify this behavior change, and the change removes support introduced by #12916. Please preserve the multi-target state, materialization, cache and SPIR-V paths and their tests while applying the scalarized image-table fix.
Non-blocking
None.
Sources
- Current-main multi-target collection at 422f96b3
- PR-head single-target limit at 6bf492e8
- Current-main target-merge vector at 422f96b3
- PR-head single target field at 6bf492e8
- Multi-target feature merged by #12916
Not verified
I did not run the author's Windows/Vulkan tests or shader replay locally, a local C++ build, Linux product tests, macOS relinker checks, or console behavior. The GPU and replay results above are the author's report.
To get this to a good state
Preserve multi-target merge handling and coverage from current main, then rerun the relevant multi-target regressions and the captured shader preparation comparison on the revised head. I will repeat the same source, diff, conventions and merge-preview checks on that head.
|
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.
|
|
@Prophet069 thanks for these. One problem common to all five PRs (#13145, #13146, #13147, #13148, #13149): each commit sits on main For this PR, |
An image-table offset that is scaled by a constant stride before v_readfirstlane is recognized as a constant-stride column through selects whose condition the active mask implies. Runtime SRT evaluation supports unsigned greater-than-or-equal comparisons, uniform selected components of constructed U32x4 values and known active-mask conjuncts in nested selects. Image dimension queries stay in GPU IR and variable scales are not constant-stride columns. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
6bf492e to
74952fa
Compare
|
Thanks, you were right: I squashed onto a moving @MarcZX10 The single-target |
|
@Prophet069 a heads-up: this branch sets the files #12916 changed back to their contents before #12916 merged ( AI-assisted: yes (Claude Code) |
MarcZX10
left a comment
There was a problem hiding this comment.
Re-checked — new head, looks good
The current head preserves the multi-target merge behavior that was missing from the earlier head, and I found no blocking issue in the revised scalarized image-table change.
What I ran
Reviewed head 74952faffa809977ec9b690d38dfbe95f4bdc770 on base 8c8b9986204d5b1cbb23847b363df00ae832ec9c; the merge preview is clean (c2c1c5de3736686f39aace3ea02a5f3ccd05257b). Environment: Linux x86-64. No local build or tests were run.
git diff --check 8c8b9986204d5b1cbb23847b363df00ae832ec9c...74952faffa809977ec9b690d38dfbe95f4bdc770
git merge-tree --write-tree 8c8b9986204d5b1cbb23847b363df00ae832ec9c 74952faffa809977ec9b690d38dfbe95f4bdc770
python3 tools/check_conventions.py --base 8c8b9986204d5b1cbb23847b363df00ae832ec9c --head 74952faffa809977ec9b690d38dfbe95f4bdc770
python3 tools/pr_overlap.py --branch 74952faffa809977ec9b690d38dfbe95f4bdc770 --base main --repo boykopovar/AnyPS5 --jobs 16
gh pr checks 13149 --repo boykopovar/AnyPS5Results
| Check | Result |
|---|---|
| Diff check | Passed; no output |
| Merge preview | Clean |
| Conventions on exact PR head | Passed |
| Open-PR overlap scan | No confirmed overlapping hunks; 31 comparisons were inconclusive because those candidate PRs conflict with the base. I manually checked the relevant shared files in #12548, #12828 and #12764; their overlapping evaluator/test/documentation changes are in separate cases or entries. |
| Hosted checks | Windows, Ubuntu/Windows relinker, relinker ASan, conventions, eligibility, labels and report passed. Linux, register-context and write-watch were skipped. Both macOS relinker jobs were pending when checked. |
| Author-reported focused tests | 63/63 on main and the new head, plus the two-target blend regression on both; same Windows/RTX 3080 environment, no VUIDs reported. Not independently run. |
| Local build/tests | Not run; no build performed |
Unedited output
git diff --check 8c8b9986204d5b1cbb23847b363df00ae832ec9c...74952faffa809977ec9b690d38dfbe95f4bdc770
(no output; exit status 0)
git merge-tree --write-tree 8c8b9986204d5b1cbb23847b363df00ae832ec9c 74952faffa809977ec9b690d38dfbe95f4bdc770
c2c1c5de3736686f39aace3ea02a5f3ccd05257b
python3 tools/check_conventions.py --base 8c8b9986204d5b1cbb23847b363df00ae832ec9c --head 74952faffa809977ec9b690d38dfbe95f4bdc770
All conventions checks passed.
python3 tools/pr_overlap.py --branch 74952faffa809977ec9b690d38dfbe95f4bdc770 --base main --repo boykopovar/AnyPS5 --jobs 16
<!-- pr-overlap -->
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](https://www.github.com/boykopovar/AnyPS5/pull/5021) | `docs/dev/debt/shader.md` |
| [PR 5371](https://www.github.com/boykopovar/AnyPS5/pull/5371) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp`<br>`docs/dev/debt/shader.md` |
| [PR 5609](https://www.github.com/boykopovar/AnyPS5/pull/5609) | `core/shader/recompiler/Optimization/src/SrtWalker/SrtEvaluator.cpp` |
| [PR 6235](https://www.github.com/boykopovar/AnyPS5/pull/6235) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp`<br>`docs/dev/debt/shader.md` |
| [PR 6280](https://www.github.com/boykopovar/AnyPS5/pull/6280) | `core/shader/recompiler/Optimization/src/ResourceTracker.cpp`<br>`docs/dev/debt/shader.md` |
| [PR 6412](https://www.github.com/boykopovar/AnyPS5/pull/6412) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp`<br>`docs/dev/debt/shader.md` |
| [PR 6488](https://www.github.com/boykopovar/AnyPS5/pull/6488) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp`<br>`core/shader/recompiler/Optimization/src/ResourceTracker.cpp`<br>`docs/dev/debt/shader.md` |
| [PR 6924](https://www.github.com/boykopovar/AnyPS5/pull/6924) | `docs/dev/debt/shader.md` |
| [PR 7392](https://www.github.com/boykopovar/AnyPS5/pull/7392) | `docs/dev/debt/shader.md` |
| [PR 7642](https://www.github.com/boykopovar/AnyPS5/pull/7642) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp`<br>`core/shader/recompiler/Optimization/src/ResourceTracker.cpp` |
| [PR 7684](https://www.github.com/boykopovar/AnyPS5/pull/7684) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp`<br>`core/shader/recompiler/Optimization/src/ResourceTracker.cpp`<br>`docs/dev/debt/shader.md` |
| [PR 7922](https://www.github.com/boykopovar/AnyPS5/pull/7922) | `docs/dev/debt/shader.md` |
| [PR 8004](https://www.github.com/boykopovar/AnyPS5/pull/8004) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp`<br>`docs/dev/debt/shader.md` |
| [PR 9020](https://www.github.com/boykopovar/AnyPS5/pull/9020) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp`<br>`docs/dev/debt/shader.md` |
| [PR 9335](https://www.github.com/boykopovar/AnyPS5/pull/9335) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp`<br>`docs/dev/debt/shader.md` |
| [PR 9356](https://www.github.com/boykopovar/AnyPS5/pull/9356) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp` |
| [PR 9387](https://www.github.com/boykopovar/AnyPS5/pull/9387) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp`<br>`docs/dev/debt/shader.md` |
| [PR 9748](https://www.github.com/boykopovar/AnyPS5/pull/9748) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp`<br>`docs/dev/debt/shader.md` |
| [PR 9889](https://www.github.com/boykopovar/AnyPS5/pull/9889) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp`<br>`core/shader/recompiler/Optimization/src/ResourceTracker.cpp`<br>`docs/dev/debt/shader.md` |
| [PR 9897](https://www.github.com/boykopovar/AnyPS5/pull/9897) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp`<br>`core/shader/recompiler/Optimization/src/ResourceTracker.cpp` |
| [PR 10193](https://www.github.com/boykopovar/AnyPS5/pull/10193) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp`<br>`core/shader/recompiler/Optimization/src/ResourceTracker.cpp` |
| [PR 10233](https://www.github.com/boykopovar/AnyPS5/pull/10233) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp`<br>`docs/dev/debt/shader.md` |
| [PR 10547](https://www.github.com/boykopovar/AnyPS5/pull/10547) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp` |
| [PR 10652](https://www.github.com/boykopovar/AnyPS5/pull/10652) | `docs/dev/debt/shader.md` |
| [PR 10757](https://www.github.com/boykopovar/AnyPS5/pull/10757) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp` |
| [PR 11351](https://www.github.com/boykopovar/AnyPS5/pull/11351) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp` |
| [PR 11551](https://www.github.com/boykopovar/AnyPS5/pull/11551) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp` |
| [PR 12324](https://www.github.com/boykopovar/AnyPS5/pull/12324) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp` |
| [PR 12548](https://www.github.com/boykopovar/AnyPS5/pull/12548) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp`<br>`core/shader/recompiler/Optimization/src/SrtWalker/SrtEvaluator.cpp`<br>`core/shader/recompiler/Optimization/src/SrtWalker/SrtInstructionPredicates.cpp`<br>`docs/dev/debt/shader.md` |
| [PR 12764](https://www.github.com/boykopovar/AnyPS5/pull/12764) | `docs/dev/debt/shader.md` |
| [PR 12828](https://www.github.com/boykopovar/AnyPS5/pull/12828) | `core/libs/prx/libSceAgcDriver/tests/ShaderMemory.cpp` |
gh pr checks 13149 --repo boykopovar/AnyPS5
linux\tskipping\t0\thttps://github.com/boykopovar/AnyPS5/actions/runs/41007774550/job/123906528372
register-context\tskipping\t0\thttps://github.com/boykopovar/AnyPS5/actions/runs/41007774550/job/123906527454
relinker (ubuntu-latest, g++)\tpass\t2m43s\thttps://github.com/boykopovar/AnyPS5/actions/runs/41007774550/job/123906526464
relinker (windows-latest, g++)\tpass\t7m20s\thttps://github.com/boykopovar/AnyPS5/actions/runs/41007774550/job/123906526407
windows\tpass\t4m47s\thttps://github.com/boykopovar/AnyPS5/actions/runs/41007774550/job/123906526357
write-watch\tskipping\t0\thttps://github.com/boykopovar/AnyPS5/actions/runs/41007774550/job/123906527937
relinker-asan\tpass\t2m41s\thttps://github.com/boykopovar/AnyPS5/actions/runs/41007774550/job/123906526436
relinker (macos-latest, clang++)\tpending\t0\thttps://github.com/boykopovar/AnyPS5/actions/runs/41007774550/job/123906526467
relinker (macos-latest, clang++, x86_64)\tpending\t0\thttps://github.com/boykopovar/AnyPS5/actions/runs/41007774550/job/123906526413
conventions\tpass\t13s\thttps://github.com/boykopovar/AnyPS5/actions/runs/41007774250/job/123906463794
eligibility\tpass\t7s\thttps://github.com/boykopovar/AnyPS5/actions/runs/41007774550/job/123906464931
labels\tpass\t7s\thttps://github.com/boykopovar/AnyPS5/actions/runs/41007774317/job/123906502619
report\tpass\t51s\thttps://github.com/boykopovar/AnyPS5/actions/runs/41007775466/job/123906463951
Findings
Blocking
None on the current head. The earlier multi-target finding applied to the stale head 6bf492e804cefd0ef70ab4f4a062aeaa5eeed02d; this head has no changes to State.cpp or the recompiler merge-state files, and the author reports that the two-target regression test passes on main and this branch.
Non-blocking
No further issue found in the reviewed changes. Linux product/specialized jobs were skipped, macOS relinker checks were pending, and GPU-focused tests were not independently run here.
Sources
- Current SRT evaluator changes at this head
- Current runtime validator changes at this head
- Current regression coverage at this head
Not verified
I did not run a local build, CTest, Linux product tests, console behavior, or GPU validation. The 63/63 tests and the two-target blend test are the author's report, not a local run. The two macOS relinker jobs were still pending at the CI check snapshot.
Formatting correction — CI outputThe review is approved on the current head; I corrected the collapsed check excerpt because its separators were shown as literal |
✅ 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 (3, not counted against this PR)Validation IDs already present on main (3 IDs in 387 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. |
|
Independent Linux / AMD RADV focused validation of head Host: CachyOS Linux Method: Same machine, build directory and exact CTest selection on both trees, serial
The selection covers all entries invoking Existing warnings, not hidden: Both logs contain 30 Scope: This is focused regression/build coverage, including real GPU rendering tests, not a game bring-up or benchmark. Some entries, including the new runtime-value checks, are CPU-side checks. I did not obtain or replay the captured PPSA02015 shader, run the game, or independently establish live GPU behavior of the new scalarized image-table path; the PR's technical-debt caveat remains applicable. No console-equivalence claim. Equivalent reproducible selectionAfter building the five targets named above, generate a CTest name list and run it on each tree with the environment described above: python3 - <<'PY'
import json
import subprocess
from pathlib import Path
targets = {
"agc_shader_memory_tests",
"agc_driver_cmask_fast_clear_tests",
"agc_driver_graphics_tests",
"agc_driver_sample_multisample_target_tests",
"agc_driver_unclamped_normalized_blend_tests",
}
data = json.loads(subprocess.check_output(
["ctest", "--test-dir", "build-main", "--show-only=json-v1"], text=True))
names = [test["name"] for test in data["tests"]
if Path(test.get("command", [""])[0]).name in targets]
assert len(names) == 66
Path("radv-focused-tests.txt").write_text("\n".join(names) + "\n")
PY
ctest --test-dir build-main --tests-from-file "$PWD/radv-focused-tests.txt" \
--parallel 1 --repeat until-fail:3 --timeout 60 --no-tests=error -VAI-assisted hardware test run (Codex). No additional PR, code changes, or performance claim. |
What
A PPSA02015 compute shader scales an image-table offset by a constant stride and then moves it through
v_readfirstlaneinside selects guarded by the lane id. Resource analysis rejected it withGetImageResource dword 0 is not a valid runtime value. The resource tracker now recognizes such an offset as a constant-stride column through selects whose condition the active mask implies. Runtime SRT evaluation gains unsigned greater-than-or-equal comparisons, uniform selected components of constructed U32x4 values and known active-mask conjuncts in nested selects. Image dimension queries stay in GPU IR, and variable scales are still not constant-stride columns. The new entry indocs/dev/debt/shader.mdrecords that the live GPU behaviour of these additions is unmeasured.Tested
ANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1andVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation(VVL vulkan-sdk-1.4.321.0; neither log containswas not found but was requested by env var VK_INSTANCE_LAYERS),ctest --test-dir build -R <the 63 tests of agc_shader_memory_tests, agc_driver_cmask_fast_clear_tests, agc_driver_graphics_tests and agc_driver_sample_multisample_target_tests> -Von main8c8b998620and on this branch, same machine and GPU: main 63/63 passed, 0VUID-; this branch 63/63 passed, 0VUID-. NewShaderMemory.cppcases cover unrelated masks, disjunctions, varying and out-of-range components and variable shifts.agc_driver_unclamped_normalized_blend(the feat(agc): merge a draw into more than one unclamped normalized color target #12916 multi-target regression,2 draws blending two targets each) passed on main and on this branch with the same validation settings, 0VUID-.agc_shader_replay --prepareof a captured PPSA02015 request (code hash3d6b9e898ab900ba, 280 code words, wave64) withANYPS5_NO_SHADER_CACHE=1: on main8c8b998620it exits 1 withresource analysis failed: GetImageResource dword 0 is not a valid runtime value; chain: LoadAddressU32@pc=0x00000208(…ReadFirstLane(SelectU32(ULessThan32(GetBuiltin…; on this branch it exits 0 withprepared: 133909 SPIR-V words. The request contains game code and is not attached.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