Skip to content

fix(agc): abandon WAIT_REG_MEM after timeout under skip-failed - #6252

Open
mazen320 wants to merge 2 commits into
boykopovar:mainfrom
mazen320:fix/wait-reg-mem-skip-timeout
Open

mazen320 wants to merge 2 commits into
boykopovar:mainfrom
mazen320:fix/wait-reg-mem-skip-timeout

Conversation

@mazen320

Copy link
Copy Markdown
Contributor

What

Under APS5_SKIP_FAILED_SHADERS, a WAIT_REG_MEM that hits APS5_GPU_WAIT_TIMEOUT_MS now returns instead of spinning forever.

Previously, once an address had been reported, a later wait on the same address hit the static reported set, took continue with warned already true, and never left the poll loop. That freezes the queue worker after a hung batch leaves the wait address hot. Default (non-skip-failed) behaviour is unchanged: timeout remains diagnostic-only per existing debt.

Debt entry for WAIT_REG_MEM updated to record the skip-failed return.

Tested

Windows 11, RTX 3080. Conventions check against current main passed. No new unit test: the hang is in the queue-worker poll loop over a live GPU wait address and needs a hung batch to reproduce; exercised under bring-up with APS5_SKIP_FAILED_SHADERS where duplicate-address waits no longer pin the worker.

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 (bring-up escape hatch under APS5_SKIP_FAILED_SHADERS)
  • GPUs tested: n/a (control-flow change in wait poll; no GPU test delta)
  • AI-assisted: yes

@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 2434 docs/dev/debt/libSceAgcDriver.md
PR 4448 docs/dev/debt/libSceAgcDriver.md
PR 4449 docs/dev/debt/libSceAgcDriver.md
PR 5021 core/libs/prx/libSceAgcDriver/Execution/src/Driver/Synchronization/WaitMemory.cpp
PR 5058 docs/dev/debt/libSceAgcDriver.md
PR 5372 docs/dev/debt/libSceAgcDriver.md
PR 5499 docs/dev/debt/libSceAgcDriver.md
PR 5925 docs/dev/debt/libSceAgcDriver.md
PR 6146 docs/dev/debt/libSceAgcDriver.md
PR 6194 docs/dev/debt/libSceAgcDriver.md
PR 6218 docs/dev/debt/libSceAgcDriver.md
PR 6223 docs/dev/debt/libSceAgcDriver.md
PR 6273 docs/dev/debt/libSceAgcDriver.md
PR 6280 docs/dev/debt/libSceAgcDriver.md
PR 6300 docs/dev/debt/libSceAgcDriver.md
PR 6412 docs/dev/debt/libSceAgcDriver.md
PR 6457 docs/dev/debt/libSceAgcDriver.md
PR 6479 docs/dev/debt/libSceAgcDriver.md
PR 6481 docs/dev/debt/libSceAgcDriver.md
PR 6498 docs/dev/debt/libSceAgcDriver.md
PR 6548 docs/dev/debt/libSceAgcDriver.md
PR 6549 docs/dev/debt/libSceAgcDriver.md
PR 6557 docs/dev/debt/libSceAgcDriver.md
PR 6572 docs/dev/debt/libSceAgcDriver.md
PR 6588 docs/dev/debt/libSceAgcDriver.md
PR 6612 docs/dev/debt/libSceAgcDriver.md
PR 6636 docs/dev/debt/libSceAgcDriver.md
PR 6639 docs/dev/debt/libSceAgcDriver.md
PR 6683 docs/dev/debt/libSceAgcDriver.md
PR 6727 docs/dev/debt/libSceAgcDriver.md
PR 6730 docs/dev/debt/libSceAgcDriver.md
PR 6741 docs/dev/debt/libSceAgcDriver.md
PR 6743 docs/dev/debt/libSceAgcDriver.md
PR 6801 docs/dev/debt/libSceAgcDriver.md
PR 6804 docs/dev/debt/libSceAgcDriver.md
PR 6808 docs/dev/debt/libSceAgcDriver.md
PR 6867 docs/dev/debt/libSceAgcDriver.md
PR 6868 docs/dev/debt/libSceAgcDriver.md
PR 6887 docs/dev/debt/libSceAgcDriver.md
PR 6926 docs/dev/debt/libSceAgcDriver.md
PR 6941 docs/dev/debt/libSceAgcDriver.md
PR 6946 docs/dev/debt/libSceAgcDriver.md
PR 6947 docs/dev/debt/libSceAgcDriver.md
PR 6959 docs/dev/debt/libSceAgcDriver.md
PR 6965 docs/dev/debt/libSceAgcDriver.md
PR 6973 docs/dev/debt/libSceAgcDriver.md
PR 6977 docs/dev/debt/libSceAgcDriver.md
PR 6987 docs/dev/debt/libSceAgcDriver.md
PR 6989 docs/dev/debt/libSceAgcDriver.md
PR 6995 docs/dev/debt/libSceAgcDriver.md
PR 7002 docs/dev/debt/libSceAgcDriver.md
PR 7085 docs/dev/debt/libSceAgcDriver.md
PR 7154 docs/dev/debt/libSceAgcDriver.md
PR 7250 docs/dev/debt/libSceAgcDriver.md
PR 7254 docs/dev/debt/libSceAgcDriver.md
PR 7256 docs/dev/debt/libSceAgcDriver.md
PR 7258 docs/dev/debt/libSceAgcDriver.md
PR 7307 docs/dev/debt/libSceAgcDriver.md
PR 7310 docs/dev/debt/libSceAgcDriver.md
PR 7332 docs/dev/debt/libSceAgcDriver.md
PR 7413 docs/dev/debt/libSceAgcDriver.md
PR 7426 docs/dev/debt/libSceAgcDriver.md
PR 7432 docs/dev/debt/libSceAgcDriver.md
PR 7446 docs/dev/debt/libSceAgcDriver.md
PR 7449 docs/dev/debt/libSceAgcDriver.md
PR 7480 docs/dev/debt/libSceAgcDriver.md
PR 7505 docs/dev/debt/libSceAgcDriver.md
PR 7533 docs/dev/debt/libSceAgcDriver.md
PR 7579 docs/dev/debt/libSceAgcDriver.md
PR 7600 docs/dev/debt/libSceAgcDriver.md
PR 7604 docs/dev/debt/libSceAgcDriver.md
PR 7640 docs/dev/debt/libSceAgcDriver.md
PR 7642 docs/dev/debt/libSceAgcDriver.md
PR 7650 docs/dev/debt/libSceAgcDriver.md
PR 7659 docs/dev/debt/libSceAgcDriver.md
PR 7668 docs/dev/debt/libSceAgcDriver.md
PR 7683 docs/dev/debt/libSceAgcDriver.md
PR 7696 docs/dev/debt/libSceAgcDriver.md
PR 7717 docs/dev/debt/libSceAgcDriver.md
PR 7729 docs/dev/debt/libSceAgcDriver.md
PR 7746 docs/dev/debt/libSceAgcDriver.md
PR 7799 docs/dev/debt/libSceAgcDriver.md
PR 7800 docs/dev/debt/libSceAgcDriver.md
PR 7803 docs/dev/debt/libSceAgcDriver.md
PR 7844 docs/dev/debt/libSceAgcDriver.md
PR 7862 docs/dev/debt/libSceAgcDriver.md
PR 7869 docs/dev/debt/libSceAgcDriver.md
PR 7883 docs/dev/debt/libSceAgcDriver.md
PR 7893 docs/dev/debt/libSceAgcDriver.md
PR 7896 docs/dev/debt/libSceAgcDriver.md
PR 7910 docs/dev/debt/libSceAgcDriver.md
PR 7911 docs/dev/debt/libSceAgcDriver.md
PR 7912 docs/dev/debt/libSceAgcDriver.md
PR 7927 docs/dev/debt/libSceAgcDriver.md
PR 7948 docs/dev/debt/libSceAgcDriver.md
PR 7965 docs/dev/debt/libSceAgcDriver.md
PR 7978 docs/dev/debt/libSceAgcDriver.md
PR 7986 docs/dev/debt/libSceAgcDriver.md
PR 8014 docs/dev/debt/libSceAgcDriver.md

@mazen320

Copy link
Copy Markdown
Contributor Author

Overlap (#4855 / #5058 / #6054)

Bot flagged only docs/dev/debt/libSceAgcDriver.md (WAIT_REG_MEM debt line). Production change here is WaitMemory.cpp alone (return after timeout under APS5_SKIP_FAILED_SHADERS). No shared exports with those PRs. Depends on: none. Debt edit can rebase if their debt hunks land first.

@github-actions github-actions Bot added docs agc libSceAgc* labels Oct 15, 2026
This was referenced Oct 15, 2026
Duplicate-address reports used to continue the spin forever after the first timeout, freezing titles once a hung batch left the same wait address hot. Under APS5_SKIP_FAILED_SHADERS, return after the timeout so the queue can advance.

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix. A few things before this can go in:

  • APS5_SKIP_FAILED_SHADERS is not read anywhere on main (git grep SKIP_FAILED_SHADERS upstream/main finds nothing), so this PR would introduce that switch, not extend it. If a new bring-up switch is intended, name it for what it does (it abandons a wait, not a shader), document it where the other APS5_* environment variables are documented, and say in the description that it is new. If it comes from another branch, depend on that PR.
  • With it set, waitMemory returns as if the wait had been satisfied (WaitMemory.cpp:379-385) while the memory still does not match the reference/mask. That is the "plausible result where main keeps waiting" case; the debt line should list it under the section for deliberate deviations, and the behaviour should be limited to exactly the already-reported address you describe, not every timed-out wait.
  • No test. The hang can be reproduced on Linux CI: a queue whose WAIT_REG_MEM targets an address nobody writes, with APS5_GPU_WAIT_TIMEOUT_MS small, submitted twice on the same address (the second wait is the one that never leaves the poll loop). Please add that test to the libSceAgcDriver tests, and show it fails without the change.
  • #6660 changes the same lines (WaitMemory.cpp:376-377) and the WAIT_REG_MEM debt line; one of the two will need a rebase.

@mazen320

Copy link
Copy Markdown
Contributor Author

The continue on a repeated address only skips the diagnostic print. warned is local to that waitMemory call, so a second wait on the same address still enters the timeout block once, then continues, then keeps polling with warned set. Falling through to the print does the same thing: neither path returns. An unsatisfied wait stays in the poll until the memory matches, which is what the debt line already says.

APS5_SKIP_FAILED_SHADERS is not on main, so this patch would be adding it. Returning from the timeout treats the wait as finished while the value still does not match. Limiting that return to an address already in reported does not free the worker: the first wait on that address is the one still inside the poll, and a second wait on the same worker does not start until the first returns.

I am not going to add that switch. Abandoning a wait after the first timeout would be a different behaviour from main, and it would need a debt line that says the wait returns unsatisfied, plus a test. #6660 touches this function for address masking, not this loop.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 19, 2026

This branch has not been deployed

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

Labels

agc libSceAgc* docs needs rebase Conflicts with main, rebase needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants