Repository navigation
fix(shader): reuse completed pixel lane proofs - #6645
adriadam10 merged 1 commit into
Conversation
|
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.
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.
|
|
Hey @DangerChamp, I've been testing this same proof-cache path. There's a new reporter comparison worth linking here: the exact earlier candidate passes the NHL26 capture and added synthetic CPU controls, but the retained PRAGMATA request still exhausts its budget at 512. I haven't built your whole branch, so this isn't a run of #6645, but I don't think The next comparison I'm requesting on #6582 is the same cached proof at 2,048 steps, with the cycle-safety checks unchanged. Can we coordinate this PR's ordering with @MarcZX100's #6180 before any further code submission? I'll keep our follow-up work coordinated here rather than open another matching PR. AI-assisted investigation and write-up, independently reviewed. |
|
Rebase handoff — rebased onto current Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release,
To pick it up: Head is |
|
Coordination update from #6582: on refreshed main |
|
I tested the current #6645 implementation against the retained #6582 shader request. After rebasing its original commit onto main I prepared a branch for this PR at On Windows 11 Pro (Ryzen 9 7900X, RTX 4070 Ti SUPER, NVIDIA driver 32.0.16.1742), Release The hosted PR still points to |
adriadam10
left a comment
There was a problem hiding this comment.
Thanks for the work here. Reading the diff against the evidence in the thread, this doesn't yet fix #6582, so I can't merge it as is:
SpirvAluEmitterMath.cpp:347(Prove): a result is cached only ifcycleGenerationdidn't move while proving it. Inside a loop-carried saved EXEC mask every sub-proof crosses the Phi back edge, so none of them is cached and the loop is walked again for every use. That is the case the issue is about. Ondra97 measured it: with this commit rebased on main and the 2,048-step budget, the retained #6582 request still fails with the lane-63 refusal, and an eight-round saved-EXEC regression fails (the three-round test inShaderMemory.cpp:2306passes). Please extend the test to eight rounds so it pins what the PR claims (it has to fail on current main).- Please cache by (value, proof type) as in overlordxrz-source's
b76d8d31, with the cycle handling it settled on. Ondra97 published a repair branch for this PR that does that and keeps your commit and tests: https://github.com/Ondra97/AnyPS5/tree/codex/pr6645-repair-20261019 (99ed46c85). Taking it (or equivalent) fits better than the provisional generation counter. - The branch conflicts with main: #6180 raised the budget to 2,048 and
EntryLaneWalkstill hasbudget = 512uhere (SpirvAluEmitterMath.cpp:412). Please rebase and keep 2,048. Fixes #6582should wait until the retained request replays successfully with your change; the CPU tests alone don't show it.
Not checked by us: the Windows/NVIDIA replay of the PRAGMATA request (relying on Ondra97's report above).
|
@adriadam10 I updated the repair branch to The branch contains the requested eight-round saved-EXEC regression, caches by I repeated the comparison on that exact main and repair head:
The request is unchanged: SHA-256 Release Environment: Windows 11 Pro, Ryzen 9 7900X, 32 GB RAM, NVIDIA GeForce RTX 4070 Ti SUPER, driver These results verify the captured shader failure; I did not run the title from this branch. #6645 still points to AI-assisted integration, testing, and write-up: yes. |
Memoize successful entry-lane proofs without retaining results that depend on active Phi cycles. Add regression coverage for saved EXEC-mask loops on narrower host subgroups. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Ondra97 <Ondra97@users.noreply.github.com>
e6f33ea to
cd0c8c1
Compare
|
Took this over, since the review points were all fixable on the branch:
Ondra97 reported the retained #6582 request passes with this algorithm; full ctest on the rebase passes; merged. |
What
Fixes #6582 by memoizing completed entry-lane proofs by value and proof type, while avoiding cached results that depend on active Phi cycles.
Adds regression coverage for saved EXEC-mask loops and helper-lane safety on wave64 shaders running on 32-lane host subgroups.
Tested
Windows host with MinGW Release build:
agc_shader_memorypassedgit diff --checkpassedChecklist
main; no other open PR implements the same functions.mdfiles or images added