// Computes a minimal set of root nodes required to traverse, in the forward
// direction, the CFG represented by the given vector of blocks, and successor
-// and predecessor functions.
+// and predecessor functions. When considering adding two nodes, each having
+// predecessors, favour using the one that appears earlier on the input blocks
+// list.
std::vector<BasicBlock*> TraversalRoots(const std::vector<BasicBlock*>& blocks,
libspirv::get_blocks_func succ_func,
libspirv::get_blocks_func pred_func) {
assert(
current_block_ &&
"RegisterBlockEnd can only be called when parsing a binary in a block");
-
vector<BasicBlock*> next_blocks;
next_blocks.reserve(next_list.size());
next_blocks.push_back(&inserted_block->second);
}
+ if (current_block_->is_type(kBlockTypeLoop)) {
+ // For each loop header, record the set of its successors, and include
+ // its continue target if the continue target is not the loop header
+ // itself.
+ std::vector<BasicBlock*>& next_blocks_plus_continue_target =
+ loop_header_successors_plus_continue_target_map_[current_block_];
+ next_blocks_plus_continue_target = next_blocks;
+ // If this block is marked as Loop-type, then the continue construct is
+ // the most recently created CFG construct.
+ auto continue_target = cfg_constructs_.back().entry_block();
+ if (continue_target != current_block_) {
+ next_blocks_plus_continue_target.push_back(continue_target);
+ }
+ }
+
current_block_->RegisterBranchInstruction(branch_instruction);
current_block_->RegisterSuccessors(next_blocks);
current_block_ = nullptr;
};
}
+Function::GetBlocksFunction
+Function::AugmentedCFGSuccessorsFunctionIncludingHeaderToContinueEdge() const {
+ return [this](const BasicBlock* block) {
+ auto where = loop_header_successors_plus_continue_target_map_.find(block);
+ return where == loop_header_successors_plus_continue_target_map_.end()
+ ? AugmentedCFGSuccessorsFunction()(block)
+ : &(*where).second;
+ };
+}
+
Function::GetBlocksFunction Function::AugmentedCFGPredecessorsFunction() const {
return [this](const BasicBlock* block) {
auto where = augmented_predecessors_map_.find(block);
auto succ_func = [](const BasicBlock* b) { return b->successors(); };
auto pred_func = [](const BasicBlock* b) { return b->predecessors(); };
auto sources = TraversalRoots(ordered_blocks_, succ_func, pred_func);
- auto sinks = TraversalRoots(ordered_blocks_, pred_func, succ_func);
+
+ // For the predecessor traversals, reverse the order of blocks. This
+ // will affect the post-dominance calculation as follows:
+ // - Suppose you have blocks A and B, with A appearing before B in
+ // the list of blocks.
+ // - Also, A branches only to B, and B branches only to A.
+ // - We want to compute A as dominating B, and B as post-dominating B.
+ // By using reversed blocks for predecessor traversal roots discovery,
+ // we'll add an edge from B to the pseudo-exit node, rather than from A.
+ // All this is needed to correctly process the dominance/post-dominance
+ // constraint when A is a loop header that points to itself as its
+ // own continue target, and B is the latch block for the loop.
+ std::vector<BasicBlock*> reversed_blocks(ordered_blocks_.rbegin(),
+ ordered_blocks_.rend());
+ auto sinks = TraversalRoots(reversed_blocks, pred_func, succ_func);
// Wire up the pseudo entry block.
augmented_successors_map_[&pseudo_entry_block_] = sources;
std::function<const std::vector<BasicBlock*>*(const BasicBlock*)>;
/// Returns the block successors function for the augmented CFG.
GetBlocksFunction AugmentedCFGSuccessorsFunction() const;
+ /// Like AugmentedCFGSuccessorsFunction, but also includes a forward edge from
+ /// a loop header block to its continue target, if they are different blocks.
+ GetBlocksFunction AugmentedCFGSuccessorsFunctionIncludingHeaderToContinueEdge() const;
/// Returns the block predecessors function for the augmented CFG.
GetBlocksFunction AugmentedCFGPredecessorsFunction() const;
std::unordered_map<const BasicBlock*, std::vector<BasicBlock*>>
augmented_predecessors_map_;
+ // Maps a structured loop header to its CFG successors and also its
+ // continue target if that continue target is not the loop header
+ // itself. This might have duplicates.
+ std::unordered_map<const BasicBlock*, std::vector<BasicBlock*>>
+ loop_header_successors_plus_continue_target_map_;
+
/// The constructs that are available in this function
std::list<Construct> cfg_constructs_;
#include <algorithm>
#include <functional>
-#include <set>
+#include <map>
#include <string>
#include <tuple>
#include <unordered_map>
uint32_t back_edge_block_id;
uint32_t loop_header_block_id;
tie(back_edge_block_id, loop_header_block_id) = edge;
-
auto is_this_header = [=](Construct& c) {
return c.type() == ConstructType::kLoop &&
c.entry_block()->id() == loop_header_block_id;
const vector<pair<uint32_t, uint32_t>>& back_edges) {
/// Check all backedges target only loop headers and have exactly one
/// back-edge branching to it
- set<uint32_t> loop_headers;
+
+ // Map a loop header to blocks with back-edges to the loop header.
+ std::map<uint32_t, std::unordered_set<uint32_t>> loop_latch_blocks;
for (auto back_edge : back_edges) {
uint32_t back_edge_block;
uint32_t header_block;
<< _.getIdName(header_block)
<< ") can only be formed between a block and a loop header.";
}
- bool success;
- tie(ignore, success) = loop_headers.insert(header_block);
- if (!success) {
- // TODO(umar): List the back-edge blocks that are branching to loop
- // header
- return _.diag(SPV_ERROR_INVALID_CFG)
- << "Loop header " << _.getIdName(header_block)
- << " targeted by multiple back-edges";
- }
+ loop_latch_blocks[header_block].insert(back_edge_block);
}
// Check the loop headers have exactly one back-edge branching to it
- for (BasicBlock* block : function.ordered_blocks()) {
- if (block->reachable() && block->is_type(kBlockTypeLoop) &&
- loop_headers.count(block->id()) != 1) {
+ for (BasicBlock* loop_header : function.ordered_blocks()) {
+ if (!loop_header->reachable()) continue;
+ if (!loop_header->is_type(kBlockTypeLoop)) continue;
+ auto loop_header_id = loop_header->id();
+ auto num_latch_blocks = loop_latch_blocks[loop_header_id].size();
+ if (num_latch_blocks != 1) {
return _.diag(SPV_ERROR_INVALID_CFG)
- << "Loop with header " + _.getIdName(block->id()) +
- " is targeted by "
- << loop_headers.count(block->id())
- << " back-edges but the standard requires exactly one";
+ << "Loop header " << _.getIdName(loop_header_id)
+ << " is targeted by " << num_latch_blocks
+ << " back-edge blocks but the standard requires exactly one";
}
}
vector<const BasicBlock*> postorder;
vector<const BasicBlock*> postdom_postorder;
vector<pair<uint32_t, uint32_t>> back_edges;
+ auto ignore_block = [](cbb_ptr) {};
+ auto ignore_edge = [](cbb_ptr, cbb_ptr) {};
if (!function.ordered_blocks().empty()) {
/// calculate dominators
DepthFirstTraversal(function.first_block(),
function.AugmentedCFGSuccessorsFunction(),
- [](cbb_ptr) {},
+ ignore_block,
[&](cbb_ptr b) { postorder.push_back(b); },
- [&](cbb_ptr from, cbb_ptr to) {
- back_edges.emplace_back(from->id(), to->id());
- });
+ ignore_edge);
auto edges = libspirv::CalculateDominators(
postorder, function.AugmentedCFGPredecessorsFunction());
for (auto edge : edges) {
/// calculate post dominators
DepthFirstTraversal(function.pseudo_exit_block(),
function.AugmentedCFGPredecessorsFunction(),
- [](cbb_ptr) {},
+ ignore_block,
[&](cbb_ptr b) { postdom_postorder.push_back(b); },
- [&](cbb_ptr, cbb_ptr) {});
+ ignore_edge);
auto postdom_edges = libspirv::CalculateDominators(
postdom_postorder, function.AugmentedCFGSuccessorsFunction());
for (auto edge : postdom_edges) {
edge.first->SetImmediatePostDominator(edge.second);
}
+ /// calculate back edges.
+ DepthFirstTraversal(
+ function.pseudo_entry_block(),
+ function
+ .AugmentedCFGSuccessorsFunctionIncludingHeaderToContinueEdge(),
+ ignore_block, ignore_block, [&](cbb_ptr from, cbb_ptr to) {
+ back_edges.emplace_back(from->id(), to->id());
+ });
}
UpdateContinueConstructExitBlocks(function, back_edges);
}
}
-TEST_P(ValidateCFG, MultipleBackEdgesToLoopHeaderBad) {
+TEST_P(ValidateCFG, MultipleBackEdgeBlocksToLoopHeaderBad) {
bool is_shader = GetParam() == SpvCapabilityShader;
Block entry("entry");
Block loop("loop", SpvOpBranchConditional);
- Block cont("cont", SpvOpBranchConditional);
+ Block back0("back0");
+ Block back1("back1");
Block merge("merge", SpvOpReturn);
entry.SetBody("%cond = OpSLessThan %intt %one %two\n");
- if (is_shader) loop.SetBody("OpLoopMerge %merge %loop None\n");
+ if (is_shader) loop.SetBody("OpLoopMerge %merge %back0 None\n");
- string str = header(GetParam()) + nameOps("cont", "loop") + types_consts() +
- "%func = OpFunction %voidt None %funct\n";
+ string str = header(GetParam()) + nameOps("loop", "back0", "back1") +
+ types_consts() + "%func = OpFunction %voidt None %funct\n";
str += entry >> loop;
- str += loop >> vector<Block>({cont, merge});
- str += cont >> vector<Block>({loop, loop});
+ str += loop >> vector<Block>({back0, back1});
+ str += back0 >> loop;
+ str += back1 >> loop;
str += merge;
str += "OpFunctionEnd";
ASSERT_EQ(SPV_ERROR_INVALID_CFG, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
MatchesRegex(
- "Loop header .\\[loop\\] targeted by multiple back-edges"));
+ "Loop header .\\[loop\\] is targeted by 2 back-edge blocks "
+ "but the standard requires exactly one"))
+ << str;
} else {
ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());
}
EXPECT_THAT(getDiagnosticString(),
MatchesRegex("The continue construct with the continue target "
".\\[loop\\] is not post dominated by the "
- "back-edge block .\\[cont\\]"));
+ "back-edge block .\\[cont\\]")) << str;
} else {
ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());
}
str += "OpFunctionEnd";
CompileSuccessfully(str);
- ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());
+ EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
}
-TEST_F(ValidateCFG, LoopWithoutBackEdgesBad) {
+TEST_F(ValidateCFG, LoopWithZeroBackEdgesBad) {
+ string str = R"(
+ OpCapability Shader
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %main "main"
+ OpName %loop "loop"
+%voidt = OpTypeVoid
+%funct = OpTypeFunction %voidt
+%main = OpFunction %voidt None %funct
+%loop = OpLabel
+ OpLoopMerge %exit %exit None
+ OpBranch %exit
+%exit = OpLabel
+ OpReturn
+ OpFunctionEnd
+)";
+ CompileSuccessfully(str);
+ ASSERT_EQ(SPV_ERROR_INVALID_CFG, ValidateInstructions());
+ EXPECT_THAT(
+ getDiagnosticString(),
+ MatchesRegex("Loop header .\\[loop\\] is targeted by "
+ "0 back-edge blocks but the standard requires exactly "
+ "one"));
+}
+
+TEST_F(ValidateCFG, LoopWithBackEdgeFromUnreachableContinueConstructGood) {
string str = R"(
OpCapability Shader
OpMemoryModel Logical GLSL450
OpBranchConditional %cond %body %exit
%body = OpLabel
OpReturn
-%cont = OpLabel
- OpBranch %loop
+%cont = OpLabel ; Reachable only from OpLoopMerge ContinueTarget parameter
+ OpBranch %loop ; Should be considered a back-edge
%exit = OpLabel
OpReturn
OpFunctionEnd
)";
CompileSuccessfully(str);
- ASSERT_EQ(SPV_ERROR_INVALID_CFG, ValidateInstructions());
- EXPECT_THAT(getDiagnosticString(),
- MatchesRegex("Loop with header .\\[loop\\] is targeted by 0 "
- "back-edges but the standard requires exactly one"));
+ EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()) << getDiagnosticString();
}
TEST_P(ValidateCFG,
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()) << getDiagnosticString();
}
+TEST_P(ValidateCFG, ContinueTargetCanBeMergeBlockForNestedStructureGood) {
+ // This example is valid. It shows that the validator can't just add
+ // an edge from the loop head to the continue target. If that edge
+ // is added, then the "if_merge" block is both the continue target
+ // for the loop and also the merge block for the nested selection, but
+ // then it wouldn't be dominated by "if_head", the header block for the
+ // nested selection.
+ bool is_shader = GetParam() == SpvCapabilityShader;
+ Block entry("entry");
+ Block loop("loop");
+ Block if_head("if_head", SpvOpBranchConditional);
+ Block if_true("if_true");
+ Block if_merge("if_merge", SpvOpBranchConditional);
+ Block merge("merge", SpvOpReturn);
+
+ entry.SetBody("%cond = OpSLessThan %intt %one %two\n");
+ if (is_shader) {
+ loop.SetBody("OpLoopMerge %merge %if_merge None\n");
+ if_head.SetBody("OpSelectionMerge %if_merge None\n");
+ }
+
+ string str = header(GetParam()) + nameOps("entry", "loop", "if_head",
+ "if_true", "if_merge", "merge") +
+ types_consts() + "%func = OpFunction %voidt None %funct\n";
+
+ str += entry >> loop;
+ str += loop >> if_head;
+ str += if_head >> vector<Block>({if_true, if_merge});
+ str += if_true >> if_merge;
+ str += if_merge >> vector<Block>({loop, merge});
+ str += merge;
+ str += "OpFunctionEnd";
+
+ CompileSuccessfully(str);
+ EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()) << getDiagnosticString();
+}
+
+TEST_P(ValidateCFG, SingleLatchBlockMultipleBranchesToLoopHeader) {
+ // This test case ensures we allow both branches of a loop latch block
+ // to go back to the loop header. It still counts as a single back edge.
+ bool is_shader = GetParam() == SpvCapabilityShader;
+ Block entry("entry");
+ Block loop("loop", SpvOpBranchConditional);
+ Block latch("latch", SpvOpBranchConditional);
+ Block merge("merge", SpvOpReturn);
+
+ entry.SetBody("%cond = OpSLessThan %intt %one %two\n");
+ if (is_shader) {
+ loop.SetBody("OpLoopMerge %merge %latch None\n");
+ }
+
+ string str = header(GetParam()) + nameOps("entry", "loop", "latch", "merge") +
+ types_consts() + "%func = OpFunction %voidt None %funct\n";
+
+ str += entry >> loop;
+ str += loop >> vector<Block>({latch, merge});
+ str += latch >> vector<Block>({loop, loop}); // This is the key
+ str += merge;
+ str += "OpFunctionEnd";
+
+ CompileSuccessfully(str);
+ EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()) << str
+ << getDiagnosticString();
+}
+
+TEST_P(ValidateCFG, SingleLatchBlockHeaderContinueTargetIsItselfGood) {
+ // This test case ensures we don't count a Continue Target from a loop
+ // header to itself as a self-loop when computing back edges.
+ // Also, it detects that there is an edge from %latch to the pseudo-exit
+ // node, rather than from %loop. In particular, it detects that we
+ // have used the *reverse* textual order of blocks when computing
+ // predecessor traversal roots.
+ bool is_shader = GetParam() == SpvCapabilityShader;
+ Block entry("entry");
+ Block loop("loop");
+ Block latch("latch");
+ Block merge("merge", SpvOpReturn);
+
+ entry.SetBody("%cond = OpSLessThan %intt %one %two\n");
+ if (is_shader) {
+ loop.SetBody("OpLoopMerge %merge %loop None\n");
+ }
+
+ string str = header(GetParam()) + nameOps("entry", "loop", "latch", "merge") +
+ types_consts() + "%func = OpFunction %voidt None %funct\n";
+
+ str += entry >> loop;
+ str += loop >> latch;
+ str += latch >> loop;
+ str += merge;
+ str += "OpFunctionEnd";
+
+ CompileSuccessfully(str);
+ EXPECT_EQ(SPV_SUCCESS, ValidateInstructions()) << str
+ << getDiagnosticString();
+}
+
/// TODO(umar): Switch instructions
/// TODO(umar): Nested CFG constructs
} /// namespace
projects / platform / upstream / SPIRV-Tools.git / commitdiff