Skip to content

fix(recompiler): leave out descriptor sources behind a branch the capture decides - #3952

Merged
oneandonlydean merged 5 commits into
boykopovar:mainfrom
xXPBSXx:fix/recompiler-resource-control-flow
Oct 12, 2026
Merged

oneandonlydean merged 5 commits into
boykopovar:mainfrom
xXPBSXx:fix/recompiler-resource-control-flow

Conversation

@xXPBSXx

@xXPBSXx xXPBSXx commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

What

A compute kernel can pick a buffer or an image path from a word of its SRT and read the same address as a V# in one path and as a T# in the other. Teardown (PPSA15246) has such a copy kernel; the recompiler validated the descriptor of the path the capture does not take and threw runtime image descriptor is invalid (details in #3701).

The machinery to leave such a source out already exists (IrResourcePlan::controlFlow and the active pass in EvaluateRuntimeSourcesImpl: an inactive source is bound as zero words and not validated), but nothing outside a unit test filled controlFlow. This PR fills it:

  • ResourceTracker records, per block, the descriptor sources the block uses, and builds controlFlow from the structured block terminators (blockInfo: true/false blocks and the branch condition). A block whose terminator is neither a plain nor a conditional branch (indirect branches, loop headers) keeps all its CFG successors, so no source is dropped for a path that cannot be decided.
  • controlFlow is only set when some source is used only behind a conditional branch; otherwise the plan is unchanged.
  • Several image instructions of different dimensions on the same T# address share one descriptor source, so a source stays active as soon as one of them is taken; the tracker therefore also records the images a block uses, and an image used only behind a branch the capture does not take is bound as a null descriptor and not checked against the T#.
  • A condition that reads memory that is not accessible makes the branch "not evaluable" (both paths stay active) instead of throwing.
  • New test verifyGuardedDescriptorSources in ShaderMemory.cpp: a T# behind a branch on an SRT word is left null when the branch skips it (even if it holds garbage), still read when the branch is taken, and still rejected when invalid and taken; and two image_stores of different dimensions on one T# in the two branches compile for the dimension that matches and fail for the one that does not. It fails without the change (the branch around the image was not planned, and without the image tracking incompatible with the static runtime image interface). docs/dev/TechnicalDebt.md lists the limits.

Tested

  • Windows, MinGW GCC 15.2.0, on main (a11c8f7f7): agc_shader_memory passes with the new case and fails without the change. 37 related tests pass (agc_shader_*, agc_structurizer, agc_scalar_select_mask, agc_driver_{bindless_images,compute_user_data_calls,guarded_null_pointer,oversized_descriptor_sets,scalar_*}, shader_buffer_address); agc_driver_oversized_dispatch was not built here, so not run. tools/check_conventions.py --base upstream/main passes.
  • Teardown with this branch plus feat(libSceCesCs): add sceCesRefersUcsProfileCp1251 and sceCesMbcsUcsContextInit #3824 and feat(libSceNpAuth): implement offline auth requests #3604: the runtime image descriptor is invalid stop and the following image descriptor is incompatible with the static runtime image interface stop are both gone; the copy kernel now compiles and the title runs on to sceNpTrophy2GetGroupInfo: invalid argument and AGC graphics: CMASK fast clears of a mipmapped, 3D or array color target are unsupported. Both are unrelated to this change.
  • Not tested: Linux, any other title, a branch whose condition needs the non-clean state (EXEC/VCC), or the pipeline cache across dispatches with different mode words (the mode word is a clean slot, which the existing reuse check compares; I did not exercise that).

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 in TechnicalDebt
  • 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
  • AI-assisted: yes (Claude Code)

@oneandonlydean

Copy link
Copy Markdown
Collaborator

EvaluateRuntimeSourcesImpl now catches the evaluator's runtime_error and goes on with both paths active (SrtDescriptorEvaluation.cpp:97-101). Catching an error to take another path isn't allowed here, and this one swallows every evaluator error, not only unreadable memory. Please drop the catch, or have the evaluator return "not evaluable" for that case without throwing and update the TechnicalDebt entry that says such a branch condition throws at record time.

The new comments in ShaderMemory.cpp (verifyGuardedDescriptorSources) need to go as well.

@xXPBSXx

xXPBSXx commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

Pushed the change you asked for: the catch is gone. Conditions are evaluated with a null-root sink on the clean evaluator, and a condition that read through a null user-data pointer is "not evaluable", so every successor stays active as before. The agc_shader_memory guarded-pointer case (null pointer chain) passes without any exception handling; TechnicalDebt wording updated (a pointer from user data that is not null still throws, as before).

Tested: agc_shader_memory, the agc|shader|srt ctest subset on Windows. Two unrelated failures: agc_write_tracking (fixed by #4131 on main, passes after merging main) and agc_driver_prepared_shaders_registered_workgroup_sync, which fails the same way with the control-flow planning disabled, so it is not caused by this PR.

AI-assisted (Claude Code).

@xXPBSXx

xXPBSXx commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up: the unrelated registered_workgroup_sync failure is now tracked in #4204 (it fails on clean main 7a84b22 as well).

AI-assisted (Claude Code).

@xXPBSXx

xXPBSXx commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main 2a5eca11c (no merge commits, head d7313bb69; same four commits as before). agc_shader_memory passes on Windows after the rebase. The review point about the catch was addressed in d7313bb69 (a null-root read in a condition now makes the condition not evaluable instead of throwing; TechnicalDebt entry adjusted); please re-check when you have time.

AI-assisted (Claude Code).

@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed labels Oct 12, 2026
@oneandonlydean oneandonlydean self-assigned this Oct 12, 2026
xXPBSXx and others added 5 commits October 12, 2026 21:14
…ture decides

A compute kernel that picks a buffer or an image path from an SRT word reads
the same address as a V# in one path and as a T# in another. The resource
tracker now records which block uses each descriptor source and builds the
resource control flow from the structured block terminators, so a source whose
only uses sit behind a conditional branch with a clean, evaluable condition is
left inactive (bound as zero words, not validated) when the capture takes the
other path. A condition that reads inaccessible memory is not evaluable instead
of throwing.

PPSA15246 (Teardown) stopped on "runtime image descriptor is invalid" for the
untaken path of such a kernel.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…lanner

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…scriptor source

Several image instructions of different dimensions on the same T# address share
one descriptor source, so a source stays active as soon as one of them is taken
and the others are still checked against the T#. Record the images a block uses
as well, deactivate the images used only behind a branch the capture does not
take, and bind them as null descriptors.

PPSA15246 (Teardown) then passes the "incompatible with the static runtime
image interface" check of the three image_store variants that are not taken.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… roots

A branch condition that reads through a null user-data pointer is now
treated as not evaluable instead of being caught, so every successor
stays active.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@xXPBSXx
xXPBSXx force-pushed the fix/recompiler-resource-control-flow branch from d7313bb to 8f254e0 Compare October 12, 2026 19:24
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 12, 2026
@oneandonlydean
oneandonlydean merged commit 7ac3f46 into boykopovar:main Oct 12, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* docs shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants