From 6a73ec426f2ab4b0de8818ee65eddeb5ee2de155 Mon Sep 17 00:00:00 2001 From: STL1te Date: Tue, 29 Sep 2026 13:59:02 +0300 Subject: [PATCH] fix(shader): a selection whose arm continues merges where the other arm goes findSelectionMerge took the nearest common post-dominator of a selection's targets, computed across the loop's back edge. When one arm only continues (reaching that block in a later iteration) and the other target is shared with an enclosing selection, the shared merge was split off the header, the post-dominator stayed where it was, and splitting repeated until the budget threw. Inside a loop, an arm that reaches the post-dominator only past the continue or merge block now leaves the selection by continuing, and the merge is the other arm's target; after one split that is the header's own synthetic merge. agc_structurizer_tests builds the smallest such graph; it hit the budget before. Co-Authored-By: Claude Opus 5.5 --- core/libs/prx/libSceAgcDriver/CMakeLists.txt | 7 ++- .../libSceAgcDriver/tests/Structurizer.cpp | 53 +++++++++++++++++++ .../ControlFlow/src/Structurizer.cpp | 30 +++++++++++ 3 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 core/libs/prx/libSceAgcDriver/tests/Structurizer.cpp diff --git a/core/libs/prx/libSceAgcDriver/CMakeLists.txt b/core/libs/prx/libSceAgcDriver/CMakeLists.txt index fb36e364d..e8a37c6c6 100644 --- a/core/libs/prx/libSceAgcDriver/CMakeLists.txt +++ b/core/libs/prx/libSceAgcDriver/CMakeLists.txt @@ -158,6 +158,11 @@ target_link_libraries(agc_shader_memory_tests PRIVATE shader_recompiler_agc_driv set_target_properties(agc_shader_memory_tests PROPERTIES CXX_EXTENSIONS OFF) configure_windows_unwind(agc_shader_memory_tests) +add_executable(agc_structurizer_tests EXCLUDE_FROM_ALL tests/Structurizer.cpp) +target_link_libraries(agc_structurizer_tests PRIVATE shader_recompiler_agc_driver libc) +set_target_properties(agc_structurizer_tests PROPERTIES CXX_EXTENSIONS OFF) +configure_windows_unwind(agc_structurizer_tests) + add_executable(agc_driver_flip_tests EXCLUDE_FROM_ALL tests/Flip.cpp ) @@ -287,7 +292,7 @@ set_target_properties(agc_driver_bda_device_tests PROPERTIES CXX_EXTENSIONS OFF) configure_windows_unwind(agc_driver_bda_device_tests) if(BUILD_TESTING) - foreach(agcTest IN ITEMS agc_driver_tests agc_shader_memory_tests agc_driver_flip_tests agc_driver_pm4_tests) + foreach(agcTest IN ITEMS agc_driver_tests agc_shader_memory_tests agc_structurizer_tests agc_driver_flip_tests agc_driver_pm4_tests) string(REGEX REPLACE "_tests$" "" agcTestName "${agcTest}") set_target_properties(${agcTest} PROPERTIES EXCLUDE_FROM_ALL OFF) add_test(NAME ${agcTestName} COMMAND ${agcTest}) diff --git a/core/libs/prx/libSceAgcDriver/tests/Structurizer.cpp b/core/libs/prx/libSceAgcDriver/tests/Structurizer.cpp new file mode 100644 index 000000000..ff4ec555f --- /dev/null +++ b/core/libs/prx/libSceAgcDriver/tests/Structurizer.cpp @@ -0,0 +1,53 @@ +// A selection in a loop with an arm that continues and another target it shares with an enclosing +// selection: its merge is where the other arm goes, so splitting the shared merge terminates. +#include "ControlFlow/Structurizer.hpp" +#include +#include +#include +#include + +using namespace ShaderRecompiler; + +int main() { + // 0 -> 1 (loop header) -> 2; 2: 5 | 3; 3: 5 | 4; 4 -> 7 (continue); 5: 6 (exit) | 7; 6 returns; + // 7 -> 1. Around the back edge every path from 4 passes 5, so 5 post-dominates both selections. + const std::vector> successors{{1}, {2}, {5, 3}, {5, 4}, {7}, {6, 7}, {}, {1}}; + ControlFlowGraph graph; + graph.entryBlock = 0; + for (std::uint32_t id = 0; id < successors.size(); ++id) { + BasicBlock block; + block.id = id; + block.startProgramCounter = id * 8; + block.endProgramCounter = id * 8 + 8; + block.instructionBegin = id * 2; + block.instructionEnd = id * 2 + 2; + block.successors = successors[id]; + auto& terminator = block.terminator; + if (successors[id].empty()) { + terminator.kind = TerminatorKind::Return; + } else if (successors[id].size() == 1) { + terminator.kind = TerminatorKind::Branch; + terminator.trueBlock = successors[id][0]; + } else { + terminator.kind = TerminatorKind::ConditionalBranch; + terminator.condition = BranchCondition::SccNonZero; + terminator.trueBlock = successors[id][0]; + terminator.falseBlock = successors[id][1]; + } + graph.blocks.push_back(std::move(block)); + } + for (const auto& block : graph.blocks) { + for (const auto successor : block.successors) graph.blocks[successor].predecessors.push_back(block.id); + } + try { + Structurizer{}.Structurize(graph); + } catch (const std::exception& error) { + std::fprintf(stderr, "%s\n", error.what()); + return 1; + } + if (graph.FindBlock(2).terminator.mergeBlock == graph.FindBlock(3).terminator.mergeBlock) { + std::fprintf(stderr, "the nested selections share merge block %u\n", graph.FindBlock(2).terminator.mergeBlock); + return 1; + } + return 0; +} diff --git a/core/shader/recompiler/ControlFlow/src/Structurizer.cpp b/core/shader/recompiler/ControlFlow/src/Structurizer.cpp index 591c76217..8b7f2259f 100644 --- a/core/shader/recompiler/ControlFlow/src/Structurizer.cpp +++ b/core/shader/recompiler/ControlFlow/src/Structurizer.cpp @@ -288,6 +288,26 @@ bool canReachBefore(const ControlFlowGraph& graph, std::uint32_t start, std::uin return false; } +// Whether `start` reaches `target` without passing the loop's continue or merge block. +bool reachesWithinIteration(const ControlFlowGraph& graph, const NaturalLoop& loop, std::uint32_t start, std::uint32_t target) { + std::vector pending = {start}; + std::vector visited(graph.blocks.size(), false); + while (!pending.empty()) { + const auto blockId = pending.back(); + pending.pop_back(); + if (blockId == target) { + return true; + } + if (blockId == loop.continueBlock || blockId == loop.mergeBlock || visited[blockId]) { + continue; + } + visited[blockId] = true; + const auto& block = graph.FindBlock(blockId); + pending.insert(pending.end(), block.successors.begin(), block.successors.end()); + } + return false; +} + std::uint32_t findSelectionMerge(const ControlFlowGraph& graph, const BasicBlock& block) { const auto globalMerge = graph.FindNearestCommonPostDominator(block.terminator.trueBlock, block.terminator.falseBlock); const auto* loop = findInnermostContainingLoop(graph, block.id); @@ -325,6 +345,16 @@ std::uint32_t findSelectionMerge(const ControlFlowGraph& graph, const BasicBlock if (isLoopControlGateway(graph, *loop, falseTarget) && graph.Dominates(block.id, falseTarget) && isInsideLoopConstruct(graph, *loop, trueTarget)) { return falseTarget; } + // An arm that reaches the post-dominator only in a later iteration (around the back edge) + // leaves the selection by continuing; the merge is where the other arm goes. Splitting the + // post-dominator off would leave it where it is, and the split repeats without end. + if (globalMerge != InvalidControlFlowId) { + const bool trueJoins = reachesWithinIteration(graph, *loop, trueTarget, globalMerge); + const bool falseJoins = reachesWithinIteration(graph, *loop, falseTarget, globalMerge); + if (trueJoins != falseJoins) { + return trueJoins ? trueTarget : falseTarget; + } + } return globalMerge; }