Skip to content

test(agc): measure address passes over settled host imports - #7414

Closed
thuanquang wants to merge 1 commit into
boykopovar:mainfrom
thuanquang:test/address-passes-settled-imports
Closed

thuanquang wants to merge 1 commit into
boykopovar:mainfrom
thuanquang:test/address-passes-settled-imports

Conversation

@thuanquang

Copy link
Copy Markdown
Contributor

What

agc_driver_address_passes fails on main on Windows with a GPU since #6678 was merged:

two draws reading through the table: 2 address-based draws, 2 began a pass, 0 continued one
address passes: a draw reading through the table after another did not continue its pass: 2 address-based draws, 2 began a pass, 0 continued one

I bisected it on an RX 6700 XT over the merges since 1e2d9b369:

CI doesn't show it: the Windows jobs have no Vulkan device, and the Linux jobs never settle imports.

The test's guest blocks are 64 KiB host imports. Since #6678, the driver settles an import under 1 MiB without waiting for it, and keeps the import compared until the settle's fence signals. The draws in ReadersShareAPass come right after the imports are made, while their settles are still pending:

  • the second draw's target read takes 151 us instead of 20 us (APS5_TRACE_DRAWS);
  • it begins a pass of its own instead of continuing the first draw's pass.

With APS5_NO_DEFERRED_SETTLES=1 the same build passes 3 of 3.

The test measures how draws through the BDA table share passes (#7184), not how imports settle. So it now turns deferred settles off with SetHostImportDeferredSettles(false), as #6678 did for RunHintedImport in DccKeysFirstWrite.cpp. The function does nothing off Windows. Settles are still covered by #6678's own tests.

What this leaves open, for @EagleBearerYS to judge: on Windows, a draw over memory that was imported moments before now begins a new pass until the import's settle completes. That is the cost #6678's description accepts for not waiting.

Tested

Windows 11 26200, AMD Radeon RX 6700 XT (gfx1031), driver 32.0.21045.5002, WinLibs GCC 15.2.0 r7, Release, BUILD_TESTING=ON, ANYPS5_REQUIRE_VULKAN=1:

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
  • Fixes: none
  • Unblocks: none
  • GPUs tested: AMD Radeon RX 6700 XT (gfx1031), driver 32.0.21045.5002, Windows 11 26200
  • AI-assisted: yes (Claude Code)

🤖 Generated with Claude Code

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

Copy link
Copy Markdown
Contributor

Thanks @thuanquang for the bisect and the trace. Your reading matches #6678's code. On Windows, an import under 1 MiB of write-watched private memory now starts out compared and queues a settle. Settles are polled from host-import lookups (HostImportFor and importAllocation), at most every 100 µs and only on a thread that holds the GPU lock: one lookup submits the settle batch, and a later one, after the batch's fence signals, returns the import to the write watch. ReadersShareAPass draws right after its blocks are imported, with no lookup between that could settle them, so both draws read compared memory and the second one can't continue the first one's pass.

The test change is right. The test is about how draws through the BDA table share passes, and turning deferred settles off there is what #6678 did for RunHintedImport in DccKeysFirstWrite.cpp. Waiting for the blocks to return to the write watch before drawing would also work, but your change keeps the test about passes.

On the cost: the extra passes and compared reads last only while a new import's settle is pending, so they fall on the first draws over memory imported moments before, not on draws over memory imported a while ago. A title that maps new memory every frame would pay it every frame. We're running agc_driver_address_passes on an NVIDIA RTX 3070 tonight, with and without APS5_NO_DEFERRED_SETTLES=1, and will add the result here.

@EagleBearerYS

Copy link
Copy Markdown
Contributor

@thuanquang the NVIDIA run matches yours, and it shows something our reading above missed. On an NVIDIA GeForce RTX 3070 Laptop GPU (driver 617.42), Windows 11, with a MinGW Release build of main 3de84fa, agc_driver_address_passes fails in 3 of 3 runs with the same two lines, and passes 3 of 3 with APS5_NO_DEFERRED_SETTLES=1. agc_driver_address_passes_targets_split passes 3 of 3 either way.

Before the two lines, each failing run prints:

[gpu] storage image 0x200020000: guest memory changed while GPU results were pending; keeping the CPU's blocks

That is a bug in #6678, not only the compared window after a fresh import that we described above. On Windows, an import under 1 MiB returns to the write watch after its deferred settle, and the return marks its blocks as stored by the CPU. A storage image that took its generation before the return, and whose draw marked its GPU results pending after it, has those results dropped at its next refresh, which uploads the guest bytes instead.

#7476 fixes it: a settled import under a live storage image stays compared, as it did before #6678. With #7476, agc_driver_address_passes passes 3 of 3 on the same RTX 3070 with deferred settles on and without this PR's opt-out. So once #7476 is in, the opt-out isn't needed for the test to pass; whether to keep it, so the test stays about passes, is up to you.

@thuanquang

Copy link
Copy Markdown
Contributor Author

Closing in favour of #7476, which fixes the race behind this failure. Thanks @EagleBearerYS for finding it, and @jacob6769 for the 40-run table showing this opt-out would only have hidden it. With #7476 in, agc_driver_address_passes passes on main 3e3e8b905 on my RX 6700 XT with deferred settles on, so the test keeps covering that path.

@thuanquang thuanquang closed this Oct 18, 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.

2 participants