Skip to content

test(agc): wait for the actual image in repeated untracked stores - #6359

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
Amoenus:codex/test-untracked-store-pending-owner
Oct 17, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
Amoenus:codex/test-untracked-store-pending-owner

Conversation

@Amoenus

@Amoenus Amoenus commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

What

Fixes #6347. RunSecondStore must hold its own storage image pending before flushing the first result. Its wait combined FindLive with AnyPendingOverlaps, which can accept another overlapping image or a flush already in progress. It did not establish that the expected image was pending.

Use the existing FindPending lookup and require it to return that live image, under the existing GPU mutex. The bounded wait, result assertions and intervening CPU-write checks remain. This changes only two lines in the existing fixture.

#5708 adds a different sample-then-redirty case; its diff does not change this wait. No duplicate correction was found.

Manual overlap check: #5761 is explicitly a combined test build, not for merging, and retains the old predicate. #6188 at 0eb0d8903 adds range-list assertions inside this same wait; it does not implement the identity gate. Focused compositiona12910c4e keeps those assertions inside this PR's stricter gate and passes5/5 per physical GPU (maximum0.591s). This is only the extracted fixture hunk, not a build or validation of the full #6188 stack. Proposed merge order: #6359 first, then retain #6188's assertions on rebase.

Tested

Rebased head b15060bd9dd5539c4d1ffb9a546c9f50ac2243db, base main 5d9dcc42d2ec5401aebd089028b8d4d7410a77b4. range-diff confirms the two-line patch is unchanged; its full source tree equals the tested main5d + old-head composition 1accfe0b7. The previous published head2bc is preserved on codex/checkpoint-pr6359-before-main5d.

  • Exact rebased fixture target builds; source/executable hashes remain fixed. Five unrestricted processes per physical NVIDIA/AMD GPU pass. The separate constrained neighbour wait remains:4/5 NVIDIA and5/5 AMD fail at the result was never held pending, before this corrected wait. This is not a standalone passing-fixture claim.
  • The rebased head plus unchanged test(agc): keep waiting for the neighbour copy case's result #6385 at 704c04aa8 passes20/20 fresh processes, five per physical GPU/profile, maximum0.945s. Each process verifies the selected GPU and affinity. Conventions and whitespace checks pass; Cppcheck2.20/clang-tidy23.1.3 are clean for the changed translation unit (16.277s, no parser/configuration failure).
  • Current-main four-way comparison:20 runs per GPU/profile/build,320 processes. Main and this patch alone reproduce the neighbour-wait failure under one-CPU scheduling; test(agc): keep waiting for the neighbour copy case's result #6385 alone and both pass80/80 each. The first-result assertion is not observed in that campaign, which does not replace the older red/green evidence below. One separate main/NVIDIA live-image assertion is tracked as #6735.

Windows11 Enterprise26200, WinLibs GCC15.2.0r7 Release/bundled dependencies/SPIRV-Tools ON; physical GPUs/drivers on the checklist. Shader cache disabled; child masks0x1/0xffffffff verified. Passing runs retain four internal watch-transition omissions. This refresh is selected-target Windows coverage, not a new full app/suite, Linux, title or capability result. The former full-build and wider matrices below keep their historical source pins. Proposed landing order:#6385 first, then this PR; neither patch changes the other's wait.

Historical Windows campaigns and independent peer results

Head 2bc3a760f1889500ba5d3bc7ba2eddf1b1ad7603, base main 598dd15954f23f0c239b5c86a2e3ede53474931e. Windows 11 Enterprise 26200, WinLibs GCC 15.2.0r7, Release, bundled dependencies and SPIRV-Tools enabled:

  • Unchanged main: the existing fixture fails 5/5 on each GPU at the first result was not stored.
  • Test-only diagnostic with the original predicate: both GPUs report expected image is pending=0 immediately before that assertion. Diagnostic removed; exact proposed source restored.
  • Correction: initial 5/5 per GPU, then 100/100 serial fresh-process runs per GPU. Physical-device output checked in every repetition. Maximum 0.71s NVIDIA /0.58s AMD; 30s per-test limit, AnyPS5 shader cache disabled.
  • Thirteen nearby storage/import/write-tracking/guest-memory groups pass once per selector: 26 executions, maximum 2.18s. No CTest failures, skips or timeouts.
  • Full Windows app/libs build, patched-library validation, conventions, registration and whitespace checks pass. Source stayed clean and pinned.
  • Cppcheck 2.20 and clang-tidy 23.1.3: no selected diagnostics or parser/configuration errors in this changed translation unit; 7.568s analyzer time.

Four internal watch-transition cases still print not run, the surface is not write-watched, on main and with the correction. Zero CTest skips does not establish coverage of those cases. Our own tests above are Windows; independent Linux results are attributed below. No full-suite, new title replay or title capability result is claimed. Current-main merge preview is clean; results above retain their exact pins.

Independent Linux coverage (our own runs above remain Windows):

  • @milescoviello's earlier Ubuntu result, head 2bc3a760f merged onto main 8dc92ae29, GCC 13.3/RTX 4070 Ti driver 595.99, fails 3/3 at the neighboring-page check before RunSecondStore. It did not reach this wait.
  • @SleglJan's mode-separated follow-up, exact head/base 2bc3a760f/598dd1595, CachyOS Linux 7.2.10, GCC 16.2.1 Release, RTX 3080 driver 615.78.08, shader cache on: in unwatch, main fails 6/10 at the first result was not stored, while this PR passes 10/10. Four internal watch cases remain unrun in that mode. With watch or probe, main and this PR each fail 3/3 at the earlier neighboring-page check. This PR plus unchanged test(agc): keep waiting for the neighbour copy case's result #6385 (0aba57d29) passes 3/3 in each watched mode without internal omissions and 10/10 in unwatch. None of the 48 reported processes fails at the result was never held pending.

The combined result covers both distinct fixture corrections; it does not make #6385 part of this PR or establish title capability. These are the other contributor's reported Linux results, not a local reproduction.

Independent RTX4080 Windows confirmation from @Neithennn: comment, this head merged onto main0e4ea13a0, driver617.42, GCC15.2 Release, shader cache disabled. The #6347 first-result assertion occurs4/5 on main and0/110 with this PR. A separate NeighbourCopied first-loop failure remains (15/110 in that campaign), now tracked in #6383; those runs are not counted as passing fixture executions. This PR does not change that wait.

Checklist

  • Based on recent main; no other open PR corrects this wait
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt; none added
  • Unimplemented paths throw; no stubs or production changes
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source: n/a, none added
  • Generic behaviour, not specific to one title
  • Depends on: test(agc): keep waiting for the neighbour copy case's result #6385 (independent neighbour wait; land first for consistently passing full-fixture validation)
  • Unblocks: none; repairs a synthetic fixture used during PPSA99902 Sonic Unleashed Recompiled (PPSA99902): initial gameplay on fork and remaining work #4443 validation
  • GPUs tested: NVIDIA GeForce RTX 5080, driver610.62/32.0.16.1062; AMD Radeon(TM) Graphics (Ryzen 9950X3D integrated), driver32.0.21036.18; both Windows 11 Enterprise 26200
  • AI-assisted: yes, Codex; locally reproduced, reviewed and tested

@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

@milescoviello

Copy link
Copy Markdown
Contributor

A Linux data point, since the description lists none: Ubuntu 24.04, GCC 13.3, Release, RTX 4070 Ti (NVIDIA 595.99), this head 2bc3a760f merged onto main 8dc92ae. agc_driver_store_over_untracked still fails 3 of 3, but at a different check from #6347's:

AGC graphics: store over untracked: the result did not reach a block the CPU left alone

That is the Linux failure since #5671, reported there. This PR's change doesn't reach it on Linux.

AI-assisted: yes

@Amoenus

Amoenus commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

@milescoviello Added your exact Linux composition/result to the description. That neighboring-page assertion runs before RunSecondStore, so the corrected wait is not reached. I am keeping it separate from #6347's first-result assertion; my #5671 comment also distinguishes the two. This PR does not claim to fix the neighboring-page case or establish a passing Linux fixture. AI-assisted: Codex.

@github-actions github-actions Bot added the agc libSceAgc* label Oct 15, 2026
@Neithennn

Copy link
Copy Markdown
Contributor

Independent Windows data point.

Setup: Windows 11 Pro 26200, NVIDIA GeForce RTX 4080 (driver 617.42), AMD Ryzen 7 5800X, WinLibs GCC 15.2.0 r7, Release, bundled dependencies. Head 2bc3a760f merged onto main 0e4ea13a0. ANYPS5_NO_SHADER_CACHE=1, fresh process per run.

#6347 assertion (the first result was not stored):

  • main: 4/5 runs.
  • this PR: 0/110 runs.

A separate failure remains, on main and with this PR: the result was never held pending (1/5 on main, 15/110 with this PR, maximum run 1.05 s). A local message-only diagnostic (not kept) shows that it comes from the first loop at StoreOverUntracked.cpp:446, not from RunSecondStore. It was always the case an image copy into another block of the surface (Kind::NeighbourCopied): it missed all 256 attempts in 8 of 40 runs. This PR does not touch that path. I found no open issue or PR covering it.

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

@Amoenus

Amoenus commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor Author

@Neithennn @LeandroLP23 I refreshed the matched Windows comparison on main5d and rebased #6359's unchanged two-line patch to b15060bd9dd5539c4d1ffb9a546c9f50ac2243db. The existing validation report now has the320-process four-way table, exact fork pins and hardware/profile checks. It confirms the remaining constrained neighbour failure with #6359 alone; #6385 alone and the combination each pass80/80. The exact rebased combination passes another20/20.

The first-result assertion was not observed on this current-main campaign; its older controls retain their pins. One different main/NVIDIA assertion, the sampled surface has no live storage image, is reported separately as #6735, without a proposed cause/fix. No new game result. I listed #6385 as this PR's full-fixture validation prerequisite and propose landing it first; the source hunks remain independent.

AI-assisted: Codex.

@MarcZX10 MarcZX10 mentioned this pull request Oct 15, 2026
11 tasks done
@Neithennn

Copy link
Copy Markdown
Contributor

Thanks. The NeighbourCopied wait is fixed in #6385 (test-only); with it merged alongside #6359, agc_driver_store_over_untracked passes 100/100 on the RTX 4080. An AMD run there would be welcome.

@SleglJan

Copy link
Copy Markdown
Contributor

A Linux result for the corrected wait, split by assertion and mode as asked on #5671, also together with #6385.

  • Builds:
  • Linux 7.2.10 (CachyOS), RTX 3080 with NVIDIA 615.78.08, GCC 16.2.1, Release, BUILD_TESTING=ON, shader cache on.
  • Each run is a fresh process of agc_driver_store_over_untracked_tests.
APS5_WRITE_WATCH_IMPORTS runs per build main 598dd15 this PR this PR + #6385
watch 3 3 fail: the result did not reach a block the CPU left alone 3 fail at the same check 3 pass
probe (default) 3 3 fail at the same check 3 fail at the same check 3 pass
unwatch 10 6 fail: the first result was not stored; 4 pass 10 pass 10 pass

AI-assisted: yes (Claude Code)

@LeandroLP23

Copy link
Copy Markdown
Contributor

AMD Radeon 860M (Windows 11, driver 32.0.13058.8001), current main 5d9dcc42d, 20 fresh runs per cell, with and without pinning to one CPU: the first result was not stored didn't occur in any run, with or without this PR. The only remaining failure, the result was never held pending, is fixed by #6385, and this PR alone doesn't change it (20/20 pinned fails with it too). Table: #6385 (comment). Merged together with #6385, the test passes 20/20 pinned and 20/20 unpinned.

AI-assisted: yes (Claude Code)

TheBlenDed pushed a commit to TheBlenDed/AnyPS5 that referenced this pull request Oct 25, 2026
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.

Windows: StoreOverUntracked fails at "the first result was not stored"

6 participants