Skip to content

test(agc): keep waiting for the neighbour copy case's result - #6385

Merged
oneandonlydean merged 2 commits into
boykopovar:mainfrom
Neithennn:fix/store-over-untracked-neighbour-wait
Oct 17, 2026
Merged

oneandonlydean merged 2 commits into
boykopovar:mainfrom
Neithennn:fix/store-over-untracked-neighbour-wait

Conversation

@Neithennn

Copy link
Copy Markdown
Contributor

What

Fixes #6383. In Run(), the Kind::NeighbourCopied wait returned Outcome::Missed the first time the surface image was live while StorageTexture::AnyPending() was false. It did not check whether the result had reached guest memory. The wait now keeps polling until the surface image is live with results pending, and returns Missed only when the result is already stored, as HoldPending does. The timeout message names both conditions. Test-only change, one file.

The hypothesis written in #6383 (results already flushed when the image was first seen) was wrong. A local diagnostic (not kept) printed the state at each miss over 30 runs on unchanged main: in 3,215 of 3,218 misses the result was neither pending nor stored, and the first word of the surface still held Untouched in 2,899 of them. Only 3 misses had the result stored. Misses occurred in every run, and 4 of 30 runs reached 256.

Tested

Windows 11 Pro 26200, NVIDIA GeForce RTX 4080 (driver 617.42), AMD Ryzen 7 5800X, WinLibs GCC 15.2.0 r7, Ninja, Release, bundled dependencies, BUILD_TESTING=ON. ANYPS5_NO_SHADER_CACHE=1, fresh process per run.

No AMD or Linux run.

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
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: none
  • GPUs tested: NVIDIA GeForce RTX 4080, driver 617.42, Windows 11 Pro 26200
  • AI-assisted: yes (diagnosis, change and runs done with Claude Code)

🤖 Generated with Claude Code

The NeighbourCopied case returned Missed as soon as the surface image was
live with nothing pending, although its result had not reached guest
memory yet. It now waits until the result is pending or stored, as
HoldPending does, and only a stored result is a miss.

Fixes boykopovar#6383

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@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 6188 core/libs/prx/libSceAgcDriver/tests/execution/StoreOverUntracked.cpp
PR 6447 core/libs/prx/libSceAgcDriver/tests/execution/StoreOverUntracked.cpp

@github-actions github-actions Bot mentioned this pull request Oct 15, 2026
11 tasks done
@Amoenus

Amoenus commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Current-main Windows refresh (2026-10-16 UTC)

Independent confirmation of @LeandroLP23's AMD result, with both our physical GPUs. Base 5d9dcc42d2ec5401aebd089028b8d4d7410a77b4; unchanged #6359 patch 2bc3a760f and unchanged #6385 head 0aba57d29462365de0f394b2454919bc6d165f1e. Tested compositions: #6359 only, #6385 only, both.

Failures out of 20 fresh processes per cell, 320 total:

Build NVIDIA one CPU AMD one CPU NVIDIA unrestricted AMD unrestricted
main5d 19 20 1 0
main5d + #6359 20 20 0 0
main5d + #6385 0 0 0 0
main5d + both 0 0 0 0

Every constrained failure is the result was never held pending. The one unrestricted main/NVIDIA failure is the distinct the sampled surface has no live storage image, now #6735. No first-result assertion was observed in this campaign; many constrained main/#6359 runs stop earlier, and this is not proof that #6359's identity predicate is unnecessary. The #6385-only and combined compositions each pass all 80 processes. Main includes the newer output-lifetime/case-table fixes 9c765fa0f/d17895ed8; the historical table below retains its older pins.

Windows 11 Enterprise 10.0.26200, Ryzen 9 9950X3D, WinLibs GCC15.2.0r7, Release/bundled dependencies/SPIRV-Tools ON. RTX5080 (10de:2c02, driver610.62/32.0.16.1062) and Radeon integrated graphics (1002:13c0, driver32.0.21036.18). Each selected fixture target builds; source/executable hashes stay fixed within each composition. All processes verify the actual GPU and child affinity: 0x1 or 0xffffffff. Shader cache disabled, driver caches uncontrolled; all processes finish below30s, overall maximum5.803s. These are selected-target builds, not new full app/suite or Linux runs. Passing processes still report four internal watch-transition omissions.

I also rebased our unchanged two-line #6359 patch onto main5d as b15060bd9; its source tree equals the first tested composition. The exact new head rebuilds, passes conventions and Cppcheck/clang-tidy (16.277s, no selected diagnostics/parser failures), and passes5/5 unrestricted processes per GPU. The constrained checks still encounter the independent neighbour wait (4/5 NVIDIA and5/5 AMD). With this unchanged #6385 at 704c04aa8, another20/20 processes pass (five per GPU/profile, maximum0.945s). This adds40 exact-head checks to the320-process comparison.

Proposed landing order: #6385 first, then #6359. The two source hunks are independent; this order gives the full fixture its corrected neighbour wait before our final identity-gate validation. #6359 declares this validation prerequisite. #6735 is recorded separately; no fix or causal attribution for that intermittent assertion is claimed here. No production, title or FPS claim. AI-assisted: Codex.

Historical main0e4 comparison, preserved with its original pins

Independent Windows NVIDIA/AMD validation of 0aba57d29462365de0f394b2454919bc6d165f1e.

For a matched comparison, I applied your unchanged fixture patch to the same control I had just tested: main 0e4ea13a0bf52fccbfc44c975b25ab7541d63b03 plus #6359 (2bc3a760f). The tested composition on our fork differs from control b4111a275466d75aad260070f46025adbc30ff30 only by your patch. This is composition coverage, not a run of the unchanged PR head or latest main.

Windows 11 Enterprise 26200, Ryzen 9 9950X3D, WinLibs GCC 15.2.0 r7, Release/bundled dependencies. GPUs: RTX 5080 (Windows driver 32.0.16.1062) and AMD Radeon(TM) Graphics (32.0.21036.18). Target rebuilt successfully. Each cell below is 20 serial fresh processes, shader cache disabled, actual selected GPU verified in every process.

Scheduling GPU Control failures With #6385 failures
One logical CPU, child affinity verified as 0x1 NVIDIA 18/20 0/20
One logical CPU, child affinity verified as 0x1 AMD 20/20 0/20
Unrestricted, verified mask 0xffffffff NVIDIA 0/20 0/20
Unrestricted, verified mask 0xffffffff AMD 1/20 0/20

Every control failure was the result was never held pending. With the patch, maximum process times were 6.162 s NVIDIA / 0.758 s AMD when constrained and 0.703 s / 0.572 s unrestricted; no timeout (30 s bound). The affinity experiment changes scheduling on this host; it does not establish the scheduling conditions on the RTX 4080 reporter's machine.

All patched runs reached the final success message. Four existing write-watch cases still print not run, the surface is not write-watched; the changed neighbour-copy case is not among those omissions. No source diagnostics or production changes were added. Source and executable hashes remained unchanged throughout each campaign. No title, performance, Linux, full-suite, or #6396 result is claimed.

AI-assisted: Codex.

@Neithennn

Copy link
Copy Markdown
Contributor Author

@Amoenus Thanks for the NVIDIA/AMD runs, including the single-CPU affinity case: that covers the vendor this PR had no result for. Thanks as well to @MarcZX100 for closing #6396 in favor of this one.

@milescoviello

Copy link
Copy Markdown
Contributor

This also fixes the Linux failure I reported in #5671. That one fails with a different message, in the case that runs right after NeighbourCopied.

Linux (Ubuntu 24.04, GCC 13.3, Release, RTX 4070 Ti, NVIDIA 595.99), ANYPS5_NO_SHADER_CACHE=1, a fresh process per run, main ff23657:

Tree agc_driver_store_over_untracked_tests
main fails 5/5: the result did not reach a block the CPU left alone
main + #6385 (0aba57d) passes 15/15

A local diagnostic I didn't keep printed each case's name. On main the failure is always in the last case, a surface whose first block left the write watch with the page before it (NeighbourLeftWatch). When the loop skips NeighbourCopied, that case passes 3/3. So on Linux, the early Missed returns from NeighbourCopied leave something behind that breaks the next case. That matches your finding that the result was still Untouched at most misses.

Not checked: Windows. I also haven't looked at what exactly carries over between the two cases.

AI-assisted: yes

@MarcZX10

Copy link
Copy Markdown
Contributor

One test-specific question: this NeighbourCopied wait uses global StorageTexture::AnyPending(), but the case has also just stored source and keeps that source image live for the later CopyFrom. If the source still has pending results, its pending state can satisfy this condition before the destination surface has pending output, so the test may proceed without proving the condition it is meant to wait for. Could this use AnyPendingOverlaps on {address, address + SurfaceBytes} (as heldLive in RunSecondStore already does) or FindPending(address, SurfaceBytes)? I could not run the GPU fixture here, so this is a code-path concern rather than a reproduced failure.

@Dextroid17

Dextroid17 commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

@MarcZX100 I ran the GPU fixture on lavapipe (llvmpipe, LLVM 23.1.1) to check your AnyPending() question at the NeighbourCopied wait. Your case does happen, though rarely.

Setup: upstream/main 5d9dcc4 with this PR (0aba57d) merged on top, Release, BUILD_TESTING=ON. I added a local stderr line (not part of the PR) where the wait breaks. For both the destination ({address, address + SurfaceBytes}) and the source surface, it logs:

  • AnyPendingOverlaps, which counts images that are pending or still being flushed.
  • FindPending, which only looks at pending images (like AnyPending()), and only one based at the start of that range that covers it. For the destination, that is the surface's own image.

An env switch picks the break condition. The "load" runs had 16 busy-loop processes on the CPU.

break condition            runs  passed  Missed   wait broke with nothing
                                         retries  pending/flushing on dst
AnyPending() (PR), idle     200    200      3         0
AnyPending() (PR), load    1000   1000     19         1  (source was pending)
AnyPendingOverlaps(dst)     200    200      4         -  (by construction)
FindPending(dst), idle      200    200      0         -  (by construction)
FindPending(dst), load     1000   1000     29         -  (by construction)

What this shows:

  • In 1 of the 1000 loaded runs, the PR's AnyPending() broke the wait while nothing pending or flushing overlapped the destination. The source's image was pending at that moment, which was enough for AnyPending() to return true. The test still passed that run, but it moved on without the condition its comment describes.
  • In the other 1199 of 1200 breaks of the PR's condition, something was pending or flushing over the destination range. In 2 of those (1 idle, 1 load) there was no pending image at the destination's base, so the overlap was either the surface image while flushing or another image inside the range, such as the last-block image.
  • Both narrower conditions passed every run: AnyPendingOverlaps(dst) 200/200, FindPending(address, SurfaceBytes) 1200/1200. AnyPendingOverlaps(dst) also broke 3 times in its 200 runs with no pending image at the destination's base, the same way as above.
  • The Missed retry path is reachable under every condition, including the PR's, and it caused no failure in any run.

So I'd take your suggestion, and of the two I'd use FindPending(address, SurfaceBytes) != nullptr. Both passed every run, but in this fixture FindPending only matches the surface's own image (pending, based at address, covering SurfaceBytes). AnyPendingOverlaps on the range also accepts the last-block image or an image that is still flushing. All of this is lavapipe only, not real hardware.

AI-assisted: yes

@LeandroLP23

Copy link
Copy Markdown
Contributor

Another AMD run, on an iGPU and on current main: with each test process pinned to one logical CPU, main fails 20 of 20 at the result was never held pending, and this change alone passes 20 of 20.

Setup: Windows 11 Home 26300, AMD Radeon 860M (RDNA 3.5, 1002:1114, driver 32.0.13058.8001), Ryzen AI 7 350 (16 threads), WinLibs GCC 15.2.0, Release. Base main 5d9dcc42d, which already has 9c765fa0f and d17895ed8 in this fixture; both PRs merge onto it cleanly. ANYPS5_NO_SHADER_CACHE=1, a fresh ctest -R '^agc_driver_store_over_untracked$' per run. "Pinned" means start /affinity 1, so ctest and the test process run on CPU 0 only.

Build Unpinned, 20 runs Pinned to CPU 0, 20 runs
main 20 pass 20 fail: the result was never held pending
main + #6359 (2bc3a760f) 19 pass, 1 never held pending 20 fail: never held pending
main + this PR (0aba57d29) 20 pass 20 pass
main + #6359 + this PR 20 pass 20 pass

This matches @Amoenus's single-CPU result on the Radeon iGPU. On this machine the first result was not stored (#6347) never showed up on current main, in any of these runs. Before 9c765fa0f, on ff23657f3, it failed regularly here.

Not tested: Linux, NVIDIA.

AI-assisted: yes (Claude Code)

@Amoenus Amoenus 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.

Validated unchanged head 0aba57d merged onto main5d with both RTX5080 and Radeon integrated graphics. Your patch alone passes80/80 fresh processes across both GPUs and single-CPU/unrestricted profiles; matching main fails19/20 NVIDIA and20/20 AMD constrained runs at the pending-result wait. The #6359 combination also passes80/80, plus20/20 after its rebase. Source review agrees with continuing to wait rather than returning Missed before either result state is reached.

Exact pins, hardware, conditional omissions and the separate #6735 assertion are in the updated existing report. Selected-target Windows coverage; no new full-suite, title or Linux claim. AI-assisted: Codex.

@MarcZX10

Copy link
Copy Markdown
Contributor

Thanks for the additional AMD runs. The 20/20 pinned failure on current main and 20/20 pass with this change directly answer my scheduling question; the combined #6359 runs also show the fixes compose. I have no further question on the AnyPending() case.

The NeighbourCopied wait accepted any pending result once the surface
image was live, so a pending result of the source surface could end it.
It now waits for a pending result overlapping the surface's range.

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

Copy link
Copy Markdown
Contributor Author

@MarcZX100 @Dextroid17 Thanks, the concern is right: the global AnyPending() can be satisfied by the source surface. I measured both narrower conditions on the RTX 4080 before choosing.

Windows 11 Pro 26200, NVIDIA GeForce RTX 4080 (driver 617.42), WinLibs GCC 15.2.0 r7, Release, ANYPS5_NO_SHADER_CACHE=1, a fresh process of agc_driver_store_over_untracked_tests per run. Base: main 5d9dcc42d + #6452 (main does not build on Windows here without it). "Pinned" is start /affinity 1.

Condition at the NeighbourCopied wait Unpinned Pinned to CPU 0
main (control) 0/20 fail 20/20 fail: the result was never held pending
FindLive && AnyPending() (previous head 0aba57d29) 0/53 fail 0/53 fail
FindPending(address, SurfaceBytes) != nullptr 3/50 fail: the result was never held pending 0/50 fail
FindLive && AnyPendingOverlaps({address, address + SurfaceBytes}) 0/150 fail 0/150 fail

FindPending on the destination misses on this GPU, as the stricter predicate of #6396 did (4/100 in #6383), so I did not take it. I pushed dc2f8fb46, which keeps FindLive and replaces AnyPending() with AnyPendingOverlaps on the surface's range, the condition RunSecondStore used before #6359. The source surface is outside that range, so its pending results no longer end the wait.

The exact new head, merged onto the same base: 0/50 unpinned, 0/50 pinned; ctest -R agc_ -j 8 602/602. Conventions pass. Not run on AMD or Linux at this head.

Thanks as well to @Amoenus for the review and to @LeandroLP23 for the Radeon 860M runs.

AI-assisted: yes (runs and change done with Claude Code).

@LeandroLP23

Copy link
Copy Markdown
Contributor

Rerun of the new head dc2f8fb46 (AnyPendingOverlaps on the destination) on the same AMD Radeon 860M setup as above, merged onto main 5d9dcc42d, 20 fresh ctest runs per cell: 20/20 pass pinned to CPU 0 (where main fails 20/20) and 20/20 pass unpinned. Same result as the previous head on this machine.

AI-assisted: yes (Claude Code)

@marco-scheffler

Copy link
Copy Markdown

Linux data point on NVIDIA.

Ubuntu 24.04.4 (kernel 6.8.0-147), NVIDIA RTX 4000 SFF Ada Generation, driver 595.99.02 (open kernel module), Intel i9-14900, GCC 13.3.0, Release, bundled dependencies, SPIRV-Tools ON. Base main bc3c0fc13; patches applied as their diffs to StoreOverUntracked.cpp (#6359 at b15060bd9, this PR at dc2f8fb46). ANYPS5_GPU=RTX, ANYPS5_NO_SHADER_CACHE=1, 20 serial fresh processes per cell, no CPU pinning.

Build Pass Failures
main 0/20 11× the result was never held pending, 9× the result did not reach a block the CPU left alone
main + #6359 1/20 8× never held pending, 11× did not reach a block the CPU left alone
main + #6385 20/20 –
main + #6359 + #6385 20/20 –

the first result was not stored did not occur in any run. lavapipe on the same machine passes on main.

@oneandonlydean oneandonlydean self-assigned this Oct 17, 2026
@oneandonlydean
oneandonlydean merged commit bfcf02e into boykopovar:main Oct 17, 2026
1 check passed
@Neithennn

Copy link
Copy Markdown
Contributor Author

Thanks @oneandonlydean for the merge, and to everyone who ran it on hardware I don't have: @Amoenus (RTX 5080 and Radeon, plus the review), @LeandroLP23 (Radeon 860M), @milescoviello, @SleglJan and @marco-scheffler (Linux), and @Dextroid17 (lavapipe). Thanks as well to @MarcZX100 for spotting the global AnyPending() wait, which led to the second commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc*

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AGC: StoreOverUntracked's NeighbourCopied case intermittently misses its pending result on Windows / RTX 4080

8 participants