From 7f5cd4480dc18276e5bbbdc7e0704ab29503a703 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C5=A0l=C3=A9gl?= Date: Mon, 12 Oct 2026 08:34:30 +0200 Subject: [PATCH] fix(recompiler): give an early-return selection a merge when its parent's other arm reaches the same join A selection outside loops whose one arm only returns and whose other arm branches to a join it does not dominate gets an empty block on that edge as its merge (splitOneReturnJoin), but only when every other predecessor of the join dominates the selection or is dominated by it. When the parent selection's other arm is a region that ends in the same join, the selection has no structured merge and the shader fails to recompile. When every structurization attempt fails, Structurize now runs them again and accepts that split for any join the selection does not dominate. The split only inserts an empty block on the edge from the selection to the join. Graphs that structurize on main keep their result. A synthetic CFG and a synthetic program cover the shape; both fail on main with "conditional block N has no structured merge". Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Fnrk5Dazjb2tLooVSpdRQq --- .../prx/libSceAgcDriver/tests/ControlFlow.cpp | 23 ++++++++++++ .../libSceAgcDriver/tests/Structurizer.cpp | 7 ++++ .../include/ControlFlow/Structurizer.hpp | 5 ++- .../ControlFlow/src/Structurizer.cpp | 37 +++++++++++++------ 4 files changed, 59 insertions(+), 13 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/tests/ControlFlow.cpp b/core/libs/prx/libSceAgcDriver/tests/ControlFlow.cpp index 96708260f..a78c75f9b 100644 --- a/core/libs/prx/libSceAgcDriver/tests/ControlFlow.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/ControlFlow.cpp @@ -1119,6 +1119,29 @@ early_exit: {0x7e020280u, 0xbf06c002u, 0xbf850003u, 0x4a020081u, 0xbf068003u, 0xbf850006u, 0x7d880088u, 0xbf870001u, 0x4a020282u, 0xe0700000u, 0x80000100u, 0xbf810000u, 0xbefe0480u, 0xbf810000u}, Split::None}, + {"early exit arm beside a parent arm that reaches the same merge", R"( + v_mov_b32 v1, 0 + s_cmp_eq_u32 s2, 64 + s_cbranch_scc1 other + v_add_nc_u32 v1, 1, v0 + s_cmp_eq_u32 s3, 0 + s_cbranch_scc1 early_exit +join: + v_cmp_gt_u32 vcc, 8, v0 + s_cbranch_vccnz store + v_add_nc_u32 v1, 2, v1 +store: + buffer_store_dword v1, off, s[0:3], 0 + s_endpgm +other: + v_add_nc_u32 v1, 3, v0 + s_branch join +early_exit: + s_mov_b64 exec, 0 + s_endpgm)", + {0x7e020280u, 0xbf06c002u, 0xbf850009u, 0x4a020081u, 0xbf068003u, 0xbf850008u, 0x7d880088u, 0xbf870001u, 0x4a020282u, 0xe0700000u, + 0x80000100u, 0xbf810000u, 0x4a020083u, 0xbf82fff8u, 0xbefe0480u, 0xbf810000u}, + Split::None}, {"return block entered from the return block after it", R"( v_mov_b32 v1, 0 s_cmp_eq_u32 s2, 64 diff --git a/core/libs/prx/libSceAgcDriver/tests/Structurizer.cpp b/core/libs/prx/libSceAgcDriver/tests/Structurizer.cpp index 385d2830e..e8b1bbee0 100644 --- a/core/libs/prx/libSceAgcDriver/tests/Structurizer.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/Structurizer.cpp @@ -328,6 +328,13 @@ int main() { requireExactPostDominators(earlyReturn, "an early return inside a selection that joins its parent's merge"); requireStructuredBranches(earlyReturn, "an early return inside a selection that joins its parent's merge"); requireDominatedMerges(earlyReturn, "an early return inside a selection that joins its parent's merge"); + const std::vector> elseReturnSuccessors{{1, 2}, {4}, {3, 4}, {}, {5, 6}, {7}, {7}, {}}; + auto elseReturn = makeGraph(elseReturnSuccessors); + Structurizer{}.Structurize(elseReturn); + requireExactPostDominators(elseReturn, "an early return in one arm of a selection whose other arm reaches the same merge"); + requireStructuredBranches(elseReturn, "an early return in one arm of a selection whose other arm reaches the same merge"); + requireDominatedMerges(elseReturn, "an early return in one arm of a selection whose other arm reaches the same merge"); + requireOriginalBranches(elseReturn, elseReturnSuccessors, "an early return in one arm of a selection whose other arm reaches the same merge"); std::vector> diamonds; for (std::uint32_t diamond = 0; diamond < 50u; ++diamond) { const auto head = diamond * 3u; diff --git a/core/shader/recompiler/ControlFlow/include/ControlFlow/Structurizer.hpp b/core/shader/recompiler/ControlFlow/include/ControlFlow/Structurizer.hpp index 10dac4ee1..20cd18e3a 100644 --- a/core/shader/recompiler/ControlFlow/include/ControlFlow/Structurizer.hpp +++ b/core/shader/recompiler/ControlFlow/include/ControlFlow/Structurizer.hpp @@ -10,7 +10,8 @@ public: void Structurize(ControlFlowGraph& graph) const; private: - void structurize(ControlFlowGraph& graph, bool privatizeReturns, bool routeLeavingExits, bool routeSharedExits) const; + void structurizeAttempts(ControlFlowGraph& graph, bool relaxReturnJoins) const; + void structurize(ControlFlowGraph& graph, bool privatizeReturns, bool routeLeavingExits, bool routeSharedExits, bool relaxReturnJoins) const; void privatizeSharedReturns(ControlFlowGraph& graph) const; void computeDominatorTree(ControlFlowGraph& graph) const; void detectNaturalLoops(ControlFlowGraph& graph) const; @@ -19,7 +20,7 @@ private: void computeStronglyConnectedComponents(ControlFlowGraph& graph) const; void recomputeAnalyses(ControlFlowGraph& graph) const; void canonicalizeNaturalLoops(ControlFlowGraph& graph, bool routeLeavingExits, bool routeSharedExits) const; - void splitSharedMergeBlocks(ControlFlowGraph& graph, bool routeLeavingExits, bool routeSharedExits) const; + void splitSharedMergeBlocks(ControlFlowGraph& graph, bool routeLeavingExits, bool routeSharedExits, bool relaxReturnJoins) const; void isolateSemanticLoopHeaders(ControlFlowGraph& graph) const; void orderByDominance(ControlFlowGraph& graph) const; void clearStructuredTerminators(ControlFlowGraph& graph) const; diff --git a/core/shader/recompiler/ControlFlow/src/Structurizer.cpp b/core/shader/recompiler/ControlFlow/src/Structurizer.cpp index a5134b561..f699c1e3d 100644 --- a/core/shader/recompiler/ControlFlow/src/Structurizer.cpp +++ b/core/shader/recompiler/ControlFlow/src/Structurizer.cpp @@ -755,7 +755,7 @@ bool splitSharedMergeBlock(ControlFlowGraph& graph, std::uint32_t merge, const s return true; } -bool splitOneReturnJoin(ControlFlowGraph& graph) { +bool splitOneReturnJoin(ControlFlowGraph& graph, bool relaxReturnJoins) { for (const auto& block : graph.blocks) { if (block.terminator.kind != TerminatorKind::ConditionalBranch || findInnermostContainingLoop(graph, block.id) != nullptr || findSelectionMerge(graph, block) != InvalidControlFlowId) { continue; @@ -763,9 +763,9 @@ bool splitOneReturnJoin(ControlFlowGraph& graph) { const auto returns = [&](std::uint32_t target) { return graph.Dominates(block.id, target) && hasLinearPathToTerminal(graph, target); }; const auto joinsEnclosing = [&](std::uint32_t target) { const auto& predecessors = graph.FindBlock(target).predecessors; - return !graph.Dominates(block.id, target) && std::all_of(predecessors.begin(), predecessors.end(), [&](std::uint32_t predecessor) { + return !graph.Dominates(block.id, target) && (relaxReturnJoins || std::all_of(predecessors.begin(), predecessors.end(), [&](std::uint32_t predecessor) { return graph.Dominates(block.id, predecessor) || graph.Dominates(predecessor, block.id); - }); + })); }; const auto trueTarget = block.terminator.trueBlock; const auto falseTarget = block.terminator.falseBlock; @@ -1455,24 +1455,39 @@ void tarjanVisit(TarjanState& state, std::uint32_t blockId) { void Structurizer::Structurize(ControlFlowGraph& graph) const { const auto original = graph; try { - structurize(graph, false, false, false); + structurizeAttempts(graph, false); + } catch (const std::runtime_error&) { + const auto error = std::current_exception(); + graph = original; + try { + structurizeAttempts(graph, true); + } catch (const std::runtime_error&) { + std::rethrow_exception(error); + } + } +} + +void Structurizer::structurizeAttempts(ControlFlowGraph& graph, bool relaxReturnJoins) const { + const auto original = graph; + try { + structurize(graph, false, false, false, relaxReturnJoins); if (!hasStructuredConstructs(graph)) throw std::runtime_error("CFG does not have structured constructs"); } catch (const std::runtime_error&) { graph = original; try { - structurize(graph, true, false, false); + structurize(graph, true, false, false, relaxReturnJoins); if (!hasStructuredConstructs(graph)) throw std::runtime_error("CFG does not have structured constructs"); } catch (const std::runtime_error&) { const auto error = std::current_exception(); graph = original; try { - structurize(graph, false, true, false); + structurize(graph, false, true, false, relaxReturnJoins); if (hasStructuredConstructs(graph)) return; } catch (const std::runtime_error&) { } graph = original; try { - structurize(graph, false, true, true); + structurize(graph, false, true, true, relaxReturnJoins); } catch (const std::runtime_error&) { std::rethrow_exception(error); } @@ -1494,14 +1509,14 @@ void Structurizer::privatizeSharedReturns(ControlFlowGraph& graph) const { } } -void Structurizer::structurize(ControlFlowGraph& graph, bool privatizeReturns, bool routeLeavingExits, bool routeSharedExits) const { +void Structurizer::structurize(ControlFlowGraph& graph, bool privatizeReturns, bool routeLeavingExits, bool routeSharedExits, bool relaxReturnJoins) const { recomputeAnalyses(graph); verifyReducibility(graph); if (privatizeReturns) { privatizeSharedReturns(graph); } canonicalizeNaturalLoops(graph, routeLeavingExits, routeSharedExits); - splitSharedMergeBlocks(graph, routeLeavingExits, routeSharedExits); + splitSharedMergeBlocks(graph, routeLeavingExits, routeSharedExits, relaxReturnJoins); isolateSemanticLoopHeaders(graph); orderByDominance(graph); clearStructuredTerminators(graph); @@ -1795,13 +1810,13 @@ void Structurizer::canonicalizeNaturalLoops(ControlFlowGraph& graph, bool routeL throw std::runtime_error("CFG loop canonicalization exceeded rewrite budget"); } -void Structurizer::splitSharedMergeBlocks(ControlFlowGraph& graph, bool routeLeavingExits, bool routeSharedExits) const { +void Structurizer::splitSharedMergeBlocks(ControlFlowGraph& graph, bool routeLeavingExits, bool routeSharedExits, bool relaxReturnJoins) const { const auto originalBlockCount = static_cast(graph.blocks.size()); const auto splitBudget = std::max(16u, originalBlockCount * 4u); const auto programWords = estimatedSpirvWords(graph, allBlockIds(originalBlockCount)); SplitBudget budget{programWords, std::max(CloneWordLimit, programWords / CloneBudgetDivisor), std::max(CloneWordLimit, programWords), nextGotoVariable(graph)}; for (std::uint32_t splits = 0; splits < splitBudget; ++splits) { - if (!splitOneLoopMerge(graph) && !splitOneSelectionMerge(graph, budget, [this](ControlFlowGraph& candidate) { recomputeAnalyses(candidate); }, routeLeavingExits, routeSharedExits) && !splitOneReturnJoin(graph)) { + if (!splitOneLoopMerge(graph) && !splitOneSelectionMerge(graph, budget, [this](ControlFlowGraph& candidate) { recomputeAnalyses(candidate); }, routeLeavingExits, routeSharedExits) && !splitOneReturnJoin(graph, relaxReturnJoins)) { return; } rebuildPredecessors(graph);