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 <noreply@anthropic.com>
This commit is contained in:
STL1te 2026-09-29 13:59:02 +03:00
parent 3dec6ceda7
commit 6a73ec426f
3 changed files with 89 additions and 1 deletions

View File

@ -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})

View File

@ -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 <cstdio>
#include <exception>
#include <utility>
#include <vector>
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<std::vector<std::uint32_t>> 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;
}

View File

@ -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<std::uint32_t> pending = {start};
std::vector<bool> 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;
}