fix(recompiler): reject a loop-exit pass result that is not structured

The third structurize pass, which routes loop exits that leave the loop
before they meet, runs only when the plain and the privatized pass
throw, and its result was used without a check. Merged with #2471, it
turns some of those budget failures into graphs that spirv-val
rejects. In this one, selection 12 of the result lies in the selection
at 7 and branches to 7's merge block 13:

  {{1}, {2, 3}, {4}, {5}, {6, 7}, {}, {8, 9}, {10, 11}, {1}, {12, 4},
   {5}, {13}, {8, 14}, {15, 16}, {7}, {13}, {4}}

  block 12 exits the selection headed by block 12, but not via a
  structured exit

hasStructuredConstructs checks the third pass's result against the
structured control-flow rules of the SPIR-V specification (2.11), with
dominance and post-dominance taken over branch, merge and continue
edges. Back edges come only from a loop's continue block. A conditional
without a merge breaks or continues its innermost loop. A header
strictly dominates its merge, and a loop header its continue block. A
construct contains the merge of every header inside it and is entered
only through its header. It is left only to its own merge or to the
innermost loop's merge or continue block, and a continue construct only
to its loop's header or merge. When the result fails, the second pass's
error is reported, as when the third pass throws. The first two passes
are unchanged.

Differential fuzz with the structured-program generator and random
reducible graphs. Oracle: exact post-dominators,
requireStructuredBranches, merges dominated by their headers, the
original's executions, and spirv-val (SPIRV-Tools v2026.4).
- Against 9fa7d06a: the same outcome and output on every graph. That
  covers 100,000 random graphs at 0 words and with one block at 1000,
  and 30,000 programs at 16 and at 1000 words per block, each also with
  one block at 1000.
- Merged with #2471, the same 30,000 programs: the third pass had made
  41, 42 and 45 of them invalid (16 words, 16 with one block at 1000,
  1000 words). These end in #2471's own error again, and nothing else
  changes. The 1, 1 and 18 graphs that d59a9167 merged with #2471
  already structures invalidly come from the first two passes and stay
  as they are.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
AlxcBTW 2026-10-10 19:27:56 +02:00 committed by Scramblrrrr
parent 6500daa012
commit 4df387d0c1
2 changed files with 89 additions and 0 deletions

View File

@ -24,6 +24,7 @@ private:
void orderByDominance(ControlFlowGraph& graph) const;
void clearStructuredTerminators(ControlFlowGraph& graph) const;
void verifyReducibility(const ControlFlowGraph& graph) const;
bool hasStructuredConstructs(const ControlFlowGraph& graph) const;
};
}

View File

@ -1438,6 +1438,9 @@ void Structurizer::Structurize(ControlFlowGraph& graph) const {
} catch (const std::runtime_error&) {
std::rethrow_exception(error);
}
if (!hasStructuredConstructs(graph)) {
std::rethrow_exception(error);
}
}
}
}
@ -1822,4 +1825,89 @@ void Structurizer::verifyReducibility(const ControlFlowGraph& graph) const {
}
}
bool Structurizer::hasStructuredConstructs(const ControlFlowGraph& graph) const {
auto structural = graph;
for (auto& block : structural.blocks) {
if (block.terminator.mergeBlock != InvalidControlFlowId) {
block.successors.push_back(block.terminator.mergeBlock);
}
if (block.terminator.loopHeader) {
block.successors.push_back(block.terminator.continueBlock);
}
}
rebuildPredecessors(structural);
computeDominatorTree(structural);
computePostDominators(structural);
const auto inRegion = [&](const BasicBlock& header, std::uint32_t blockId) {
return structural.Dominates(header.id, blockId) && !structural.Dominates(header.terminator.mergeBlock, blockId);
};
const auto inContinue = [&](const BasicBlock& header, std::uint32_t blockId) {
return structural.Dominates(header.terminator.continueBlock, blockId) && structural.PostDominates(header.terminator.continueBlock, blockId);
};
const auto inConstruct = [&](const BasicBlock& header, std::uint32_t blockId) {
return inRegion(header, blockId) && !(header.terminator.loopHeader && inContinue(header, blockId));
};
const auto innermostLoop = [&](std::uint32_t blockId) {
const BasicBlock* innermost = nullptr;
for (const auto& header : graph.blocks) {
if (header.terminator.loopHeader && inRegion(header, blockId) && (innermost == nullptr || structural.Dominates(innermost->id, header.id))) {
innermost = &header;
}
}
return innermost;
};
const auto exitsLoop = [](const BasicBlock* loop, std::uint32_t target) {
return loop != nullptr && (target == loop->terminator.mergeBlock || target == loop->terminator.continueBlock);
};
for (const auto& block : graph.blocks) {
const auto& terminator = block.terminator;
for (const auto successor : block.successors) {
const auto& target = graph.FindBlock(successor);
if (structural.Dominates(successor, block.id) && (!target.terminator.loopHeader || target.terminator.continueBlock != block.id)) {
return false;
}
}
if (terminator.kind == TerminatorKind::ConditionalBranch && !terminator.loopHeader && terminator.mergeBlock == InvalidControlFlowId && terminator.trueBlock != terminator.falseBlock) {
const auto* loop = innermostLoop(block.id);
if (!exitsLoop(loop, terminator.trueBlock) && !exitsLoop(loop, terminator.falseBlock)) {
return false;
}
}
if (terminator.mergeBlock == InvalidControlFlowId) {
continue;
}
if (terminator.mergeBlock == block.id || !structural.Dominates(block.id, terminator.mergeBlock)) {
return false;
}
if (terminator.loopHeader && (terminator.continueBlock == terminator.mergeBlock || !structural.Dominates(block.id, terminator.continueBlock))) {
return false;
}
const auto* loop = terminator.loopHeader ? &block : innermostLoop(block.id);
for (const auto& member : graph.blocks) {
if (inConstruct(block, member.id)) {
if (member.id != block.id && member.terminator.mergeBlock != InvalidControlFlowId && !inConstruct(block, member.terminator.mergeBlock)) {
return false;
}
if (member.id != block.id && !std::all_of(member.predecessors.begin(), member.predecessors.end(), [&](std::uint32_t predecessor) { return inConstruct(block, predecessor); })) {
return false;
}
if (!std::all_of(member.successors.begin(), member.successors.end(), [&](std::uint32_t successor) { return inConstruct(block, successor) || successor == terminator.mergeBlock || exitsLoop(loop, successor); })) {
return false;
}
} else if (terminator.loopHeader && inContinue(block, member.id)) {
if (!std::all_of(member.successors.begin(), member.successors.end(), [&](std::uint32_t successor) { return inContinue(block, successor) || successor == block.id || successor == terminator.mergeBlock; })) {
return false;
}
}
}
if (terminator.loopHeader) {
const auto& predecessors = graph.FindBlock(terminator.continueBlock).predecessors;
if (!std::all_of(predecessors.begin(), predecessors.end(), [&](std::uint32_t predecessor) { return inConstruct(block, predecessor) || inContinue(block, predecessor); })) {
return false;
}
}
}
return true;
}
}