Skip to content

fix(shader): give a selection a merge when one arm returns through nested branches - #6405

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
fdfawef324:claude/shader-structurizer-branching-returns
Oct 19, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
fdfawef324:claude/shader-structurizer-branching-returns

Conversation

@fdfawef324

@fdfawef324 fdfawef324 commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

What

mergeBesideReturns (ControlFlow/src/Structurizer.cpp) finds the merge of a selection whose arms do not meet. It computes post-dominators with the edges into blocks that only lead to a return left out. On main a block counted as one that only leads to a return when a straight run of single-predecessor blocks starts at it and ends in a return (hasLinearPathToTerminal, linearTail). An arm whose return path branches again, such as an early return inside a nested if/else, still counted as a block of its own. The arms then had no common post-dominator, and the structurizer gave the whole program to route variables ("conditional block 0 has no structured merge").

  • A block now only leads to a return when it has one predecessor, the header dominates it and every successor only leads to a return (a block with no successor returns). The walk uses an explicit stack and keeps one result per block; a block on a cycle does not count.

Why it matters: in the route-variable form every original block becomes a guarded region, and the register phis at the region joins also carry values from paths that never reach a later use. In Resident Evil 4 (PPSA07411) pixel program 0x20762add00, the T# of an image_load became Phi(0, load through Phi(user data, compare mask)), and resource analysis refused it. The title stopped there, in sceAgcCreateInterpolantMapping_0100 -> Driver::ResolveShaderAbi on a game thread:

terminate called after throwing an instance of 'std::runtime_error'
  what():  shader 0x20762add00 stage 5 (shader_20762add00.req): ShaderRecompiler::Recompile: GetImageResource dword 0 is not a valid runtime value; chain: Phi(Immediate, LoadAddressU32@pc=0x00000214(GetAddressResource(Phi(GetUserData(Void), CompositeExtractU32x4(Ballot(LogicalAnd(IEqual32(GetBuiltin, Immediate), ULessThan32(Immediate, SelectU32))), Immediate)), Phi(GetUserData(Void), ...

Structured, the T# is the load through the user-data pointer, and the program prepares.

The title gets to this program only after other stops: main stops it at Value::setTypeMismatchHandler (#6126) and then at its NGG geometry states (#5884). The run above was a private build of main 8cd937e with #6126 and local changes that get it past the geometry states. The request the driver saved for the program (APS5_DUMP_SHADERS=1) shows the refusal on current main without the title, see Tested.

Overlap: #6025 (dominator sets as bitsets) also changes Structurizer.cpp; git merge-tree merges the two cleanly. No open pull request changes mergeBesideReturns.

Tested

Windows Server 2025 (10.0.26100), AMD Ryzen 9 5900X, NVIDIA GeForce RTX 3060 (driver 591.86), WinLibs MinGW-w64 GCC 15.2.0, Ninja, Release, BUILD_TESTING=ON.

Checklist

🤖 Generated with Claude Code

…sted branches

mergeBesideReturns ignores the edges into blocks that only lead to a return
when it looks for the merge of a selection whose arms do not meet. It only
recognized a straight run of single-predecessor blocks ending in a return,
so an arm whose return path branches again (an early return inside a
nested if/else) still counted as a block of its own, the arms had no
common post-dominator, and the whole program fell back to flag
structurization ("conditional block 0 has no structured merge").

A block now returns when it has one predecessor, is dominated by the
header and every successor returns. Flag structurization turns every
original block into a guarded region, and the register phis at the region
joins carry values from paths that never reach a later use: in Resident
Evil 4 (PPSA07411) pixel program 0x20762add00 the T# of an image_load
became Phi(0, load through Phi(user data, compare mask)) and resource
analysis refused it. Structured, the T# is the load through the user-data
pointer and the program prepares, captures and materializes.

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

Copy link
Copy Markdown
Contributor

I ran the test case this PR adds, on Linux.

Setup: head a2f497a merged into main at ff23657 (no conflicts; the PR's base is 25 commits behind), Linux, default CMake build (CMAKE_BUILD_TYPE empty, unoptimized), clean environment (env -i with only HOME, PATH and VK_ICD_FILENAMES set). ctest -R '^agc_structurizer$', 3 runs each.

  • with this PR: 3 of 3 runs pass.
  • with core/shader/recompiler/ControlFlow/src/Structurizer.cpp put back to main's version (the PR's test changes kept): 3 of 3 runs fail, at "an arm whose returns branch once a tail shared across arms is cloned: block 0 routes through a goto variable instead of a structured selection".

The same-executions check on that graph passes on main too, so main gives the same paths, only through a goto variable. I only ran the structurizer test, not a title.
I did not check Windows or macOS.

AI-assisted: yes

@github-actions github-actions Bot added shader core/shader agc libSceAgc* unblocks title Names a title ID on the Unblocks line labels Oct 16, 2026
@adriadam10 adriadam10 self-assigned this Oct 18, 2026
@adriadam10
adriadam10 merged commit 55a3ac6 into boykopovar:main Oct 19, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* shader core/shader unblocks title Names a title ID on the Unblocks line

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants