AuroraMiddleware/SPIRV-Tools
1
0
Fork 0
mirror of https://github.com/KhronosGroup/SPIRV-Tools synced 2024-12-25 01:01:04 +00:00
Code Issues Packages Projects Releases Wiki Activity

Use pseudo entry and pseudo exit blocks for dominance.

Browse Source 
For dominance calculations we use an "augmented" CFG
where we always add a pseudo-entry node that is the predecessor
in the augmented CFG to any nodes that have no predecessors in the
regular CFG.  Similarly, we add a pseudo-exit node that is the
predecessor in the augmented CFG that is a successor to any
node that has no successors in the regular CFG.

Pseudo entry and exit blocks live in the Function object.

Fixes a subtle problem where we were implicitly creating
the block_details for the pseudo-exit node since it didn't
appear in the idoms map, and yet we referenced it.  In such a case the
contents of the block details could be garbage, or zero-initialized.
That sometimes caused incorrect calculation of immediate dominators
and post-dominators.  For example, on a debug build where the details
could be zero-initialized, the dominator of an unreachable block would
be given as the pseudo-exit node.  Bizarre.

Also, enforce the rule that you must have an OpFunctionEnd to close off
the last function.
This commit is contained in:
David Neto 2016-06-24 02:14:16 -04:00
parent 1a050b1069
commit 5065227748
10 changed files with 219 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
source/val/BasicBlock.cpp
View File

@ -26,6 +26,7 @@
#include "BasicBlock.h"
#include <utility>
#include <vector>
using std::vector;
@ -75,6 +76,14 @@ void BasicBlock::RegisterBranchInstruction(SpvOp branch_instruction) {
return;
}
void BasicBlock::SetSuccessorsUnsafe(std::vector<BasicBlock*>&& others) {
successors_ = std::move(others);
}
void BasicBlock::SetPredecessorsUnsafe(std::vector<BasicBlock*>&& others) {
predecessors_ = std::move(others);
}
BasicBlock::DominatorIterator::DominatorIterator() : current_(nullptr) {}
BasicBlock::DominatorIterator::DominatorIterator(

8
source/val/BasicBlock.h
View File

@ -123,6 +123,14 @@ class BasicBlock {
/// Adds @p next BasicBlocks as successors of this BasicBlock
void RegisterSuccessors(const std::vector<BasicBlock*>& next = {});
/// Set the successors to this block, without updating other internal state,
/// and without updating the other blocks.
void SetSuccessorsUnsafe(std::vector<BasicBlock*>&& others);
/// Set the predecessors to this block, without updating other internal state,
/// and without updating the other blocks.
void SetPredecessorsUnsafe(std::vector<BasicBlock*>&& others);
/// Returns true if the id of the BasicBlock matches
bool operator==(const BasicBlock& other) const { return other.id_ == id_; }

31
source/val/Function.cpp
View File

@ -69,9 +69,13 @@ Function::Function(uint32_t id, uint32_t result_type_id,
result_type_id_(result_type_id),
function_control_(function_control),
declaration_type_(FunctionDecl::kFunctionDeclUnknown),
end_has_been_registered_(false),
blocks_(),
current_block_(nullptr),
pseudo_entry_block_(0),
pseudo_exit_block_(kInvalidId),
pseudo_entry_blocks_({&pseudo_entry_block_}),
pseudo_exit_blocks_({&pseudo_exit_block_}),
cfg_constructs_(),
variable_ids_(),
parameter_ids_() {}
@ -206,17 +210,29 @@ void Function::RegisterBlockEnd(vector<uint32_t> next_list,
next_blocks.push_back(&inserted_block->second);
}
if (branch_instruction == SpvOpReturn ||
branch_instruction == SpvOpReturnValue) {
assert(next_blocks.empty());
next_blocks.push_back(&pseudo_exit_block_);
}
current_block_->RegisterBranchInstruction(branch_instruction);
current_block_->RegisterSuccessors(next_blocks);
current_block_ = nullptr;
return;
}
void Function::RegisterFunctionEnd() {
if (!end_has_been_registered_) {
end_has_been_registered_ = true;
// Compute the successors of the pseudo-entry block, and
// the predecessors of the pseudo exit block.
vector<BasicBlock*> sources;
vector<BasicBlock*> sinks;
for (const auto b : ordered_blocks_) {
if (b->get_predecessors()->empty()) sources.push_back(b);
if (b->get_successors()->empty()) sinks.push_back(b);
}
pseudo_entry_block_.SetSuccessorsUnsafe(std::move(sources));
pseudo_exit_block_.SetPredecessorsUnsafe(std::move(sinks));
}
}
size_t Function::get_block_count() const { return blocks_.size(); }
size_t Function::get_undefined_block_count() const {
@ -231,6 +247,11 @@ vector<BasicBlock*>& Function::get_blocks() { return ordered_blocks_; }
const BasicBlock* Function::get_current_block() const { return current_block_; }
BasicBlock* Function::get_current_block() { return current_block_; }
BasicBlock* Function::get_pseudo_entry_block() { return &pseudo_entry_block_; }
const BasicBlock* Function::get_pseudo_entry_block() const {
return &pseudo_entry_block_;
}
BasicBlock* Function::get_pseudo_exit_block() { return &pseudo_exit_block_; }
const BasicBlock* Function::get_pseudo_exit_block() const {
return &pseudo_exit_block_;

54
source/val/Function.h
View File

@ -95,11 +95,14 @@ class Function {
/// Registers the end of the block
///
/// @param[in] successors_list A list of ids to the blocks successors
/// @param[in] successors_list A list of ids to the block's successors
/// @param[in] branch_instruction the branch instruction that ended the block
void RegisterBlockEnd(std::vector<uint32_t> successors_list,
SpvOp branch_instruction);
/// Registers the end of the function. This is idempotent.
void RegisterFunctionEnd();
/// Returns true if the \p id block is the first block of this function
bool IsFirstBlock(uint32_t id) const;
@ -149,12 +152,32 @@ class Function {
/// Returns the block that is currently being parsed in the binary
const BasicBlock* get_current_block() const;
/// Returns the psudo exit block
/// Returns the pseudo exit block
BasicBlock* get_pseudo_entry_block();
/// Returns the pseudo exit block
const BasicBlock* get_pseudo_entry_block() const;
/// Returns the pseudo exit block
BasicBlock* get_pseudo_exit_block();
/// Returns the psudo exit block
/// Returns the pseudo exit block
const BasicBlock* get_pseudo_exit_block() const;
/// Returns a vector with just the pseudo entry block.
/// This serves as the predecessors of each source node in the CFG when computing
/// dominators.
const std::vector<BasicBlock*>* get_pseudo_entry_blocks() const {
return &pseudo_entry_blocks_;
}
/// Returns a vector with just the pseudo exit block.
/// This serves as the successors of each sink node in the CFG when computing
/// dominators.
const std::vector<BasicBlock*>* get_pseudo_exit_blocks() const {
return &pseudo_exit_blocks_;
}
/// Prints a GraphViz digraph of the CFG of the current funciton
void printDotGraph() const;
@ -180,6 +203,9 @@ class Function {
/// The type of declaration of each function
FunctionDecl declaration_type_;
// Have we finished parsing this function?
bool end_has_been_registered_;
/// The blocks in the function mapped by block ID
std::unordered_map<uint32_t, BasicBlock> blocks_;
@ -192,9 +218,29 @@ class Function {
/// The block that is currently being parsed
BasicBlock* current_block_;
/// A pseudo exit block that is the successor to all return blocks
/// A pseudo entry block that, for the purposes of dominance analysis,
/// is considered the predecessor to any ordinary block without predecessors.
/// After the function end has been registered, its successor list consists
/// of all ordinary blocks without predecessors. It has no predecessors.
/// It does not appear in the predecessor or successor list of any
/// ordinary block.
/// It has Id 0.
BasicBlock pseudo_entry_block_;
/// A pseudo exit block that, for the purposes of dominance analysis,
/// is considered the successor to any ordinary block without successors.
/// After the function end has been registered, its predecessor list consists
/// of all ordinary blocks without successors. It has no successors.
/// It does not appear in the predecessor or successor list of any
/// ordinary block.
BasicBlock pseudo_exit_block_;
// A vector containing pseudo_entry_block_.
const std::vector<BasicBlock*> pseudo_entry_blocks_;
// A vector containing pseudo_exit_block_.
const std::vector<BasicBlock*> pseudo_exit_blocks_;
/// The constructs that are available in this function
std::list<Construct> cfg_constructs_;

1
source/val/ValidationState.cpp
View File

@ -353,6 +353,7 @@ spv_result_t ValidationState_t::RegisterFunctionEnd() {
assert(in_block() == false &&
"RegisterFunctionParameter can only be called when parsing the binary "
"ouside of a block");
get_current_function().RegisterFunctionEnd();
in_function_ = false;
return SPV_SUCCESS;
}

4
source/validate.cpp
View File

@ -185,6 +185,10 @@ spv_result_t spvValidate(const spv_const_context context,
binary->wordCount, setHeader,
ProcessInstruction, pDiagnostic));
if (vstate.in_function_body())
return vstate.diag(SPV_ERROR_INVALID_LAYOUT)
<< "Missing OpFunctionEnd at end of module.";
// TODO(umar): Add validation checks which require the parsing of the entire
// module. Use the information from the ProcessInstruction pass to make the
// checks.

11
source/validate.h
View File

@ -60,6 +60,12 @@ using get_blocks_func =
/// @brief Calculates dominator edges for a set of blocks
///
/// Computes dominators using the algorithm of Cooper, Harvey, and Kennedy
/// "A Simple, Fast Dominance Algorithm", 2001.
///
/// The algorithm assumes there is a unique root node (a node without predecessors),
/// and it is therefore at the end of the postorder vector.
///
/// This function calculates the dominator edges for a set of blocks in the CFG.
/// Uses the dominator algorithm by Cooper et al.
///
@ -68,7 +74,10 @@ using get_blocks_func =
/// @param[in] predecessor_func Function used to get the predecessor nodes of a
/// block
///
/// @return a set of dominator edges represented as a pair of blocks
/// @return the dominator tree of the graph, as a vector of pairs of nodes.
/// The first node in the pair is a node in the graph. The second node in the pair
/// is its immediate dominator in the sense of Cooper et.al., where a block without
/// predecessors (such as the root node) is its own immediate dominator.
std::vector<std::pair<BasicBlock*, BasicBlock*>> CalculateDominators(
const std::vector<const BasicBlock*>& postorder,
get_blocks_func predecessor_func);

133
source/validate_cfg.cpp
View File

@ -86,14 +86,15 @@ bool FindInWorkList(const vector<block_info>& work_list, uint32_t id) {
return false;
}
/// @brief Depth first traversal starting from the \p entry BasicBlock
///
/// This function performs a depth first traversal from the \p entry
/// BasicBlock and calls the pre/postorder functions when it needs to process
/// the node in pre order, post order. It also calls the backedge function
/// when a back edge is encountered
/// when a back edge is encountered.
///
/// @param[in] entry The root BasicBlock of a CFG tree
/// @param[in] entry The root BasicBlock of a CFG
/// @param[in] successor_func A function which will return a pointer to the
/// successor nodes
/// @param[in] preorder A function that will be called for every block in a
@ -102,22 +103,24 @@ bool FindInWorkList(const vector<block_info>& work_list, uint32_t id) {
/// CFG following postorder traversal semantics
/// @param[in] backedge A function that will be called when a backedge is
/// encountered during a traversal
/// NOTE: The @p successor_func return a pointer to a collection such that
/// iterators to that collection remain valid for the lifetime of the algorithm
void DepthFirstTraversal(const BasicBlock& entry,
/// NOTE: The @p successor_func and predecessor_func each return a pointer to a
/// collection such that iterators to that collection remain valid for the lifetime
/// of the algorithm
void DepthFirstTraversal(const BasicBlock* entry,
get_blocks_func successor_func,
function<void(cbb_ptr)> preorder,
function<void(cbb_ptr)> postorder,
function<void(cbb_ptr, cbb_ptr)> backedge) {
vector<cbb_ptr> out;
unordered_set<uint32_t> processed;
/// NOTE: work_list is the sequence of nodes from the entry node to the node
/// NOTE: work_list is the sequence of nodes from the root node to the node
/// being processed in the traversal
vector<block_info> work_list;
work_list.reserve(10);
work_list.push_back({&entry, begin(*successor_func(&entry))});
preorder(&entry);
work_list.push_back({entry, begin(*successor_func(entry))});
preorder(entry);
processed.insert(entry->get_id());
while (!work_list.empty()) {
block_info& top = work_list.back();
@ -140,19 +143,9 @@ void DepthFirstTraversal(const BasicBlock& entry,
}
}
/// Returns the successor of a basic block.
/// NOTE: This will be passed as a function pointer to when calculating
/// the dominator and post dominator
const vector<BasicBlock*>* successor(const BasicBlock* b) {
return b->get_successors();
}
const vector<BasicBlock*>* predecessor(const BasicBlock* b) {
return b->get_predecessors();
}
} // namespace
vector<pair<BasicBlock*, BasicBlock*>> CalculateDominators(
const vector<cbb_ptr>& postorder, get_blocks_func predecessor_func) {
struct block_detail {
@ -170,21 +163,20 @@ vector<pair<BasicBlock*, BasicBlock*>> CalculateDominators(
bool changed = true;
while (changed) {
changed = false;
for (auto b = postorder.rbegin() + 1; b != postorder.rend(); b++) {
const vector<BasicBlock*>* predecessors = predecessor_func(*b);
for (auto b = postorder.rbegin() + 1; b != postorder.rend(); ++b) {
const vector<BasicBlock*>& predecessors = *predecessor_func(*b);
// first processed/reachable predecessor
auto res = find_if(begin(*predecessors), end(*predecessors),
auto res = find_if(begin(predecessors), end(predecessors),
[&idoms, undefined_dom](BasicBlock* pred) {
return idoms[pred].dominator != undefined_dom &&
pred->is_reachable();
return idoms[pred].dominator != undefined_dom;
});
if (res == end(*predecessors)) continue;
BasicBlock* idom = *res;
if (res == end(predecessors)) continue;
const BasicBlock* idom = *res;
size_t idom_idx = idoms[idom].postorder_index;
// all other predecessors
for (auto p : *predecessors) {
if (idom == p || p->is_reachable() == false) continue;
for (const auto* p : predecessors) {
if (idom == p) continue;
if (idoms[p].dominator != undefined_dom) {
size_t finger1 = idoms[p].postorder_index;
size_t finger2 = idom_idx;
@ -216,14 +208,6 @@ vector<pair<BasicBlock*, BasicBlock*>> CalculateDominators(
return out;
}
void UpdateImmediateDominators(
const vector<pair<bb_ptr, bb_ptr>>& dom_edges,
function<void(BasicBlock*, BasicBlock*)> set_func) {
for (auto& edge : dom_edges) {
set_func(get<0>(edge), get<1>(edge));
}
}
void printDominatorList(const BasicBlock& b) {
std::cout << b.get_id() << " is dominated by: ";
const BasicBlock* bb = &b;
@ -408,35 +392,71 @@ spv_result_t PerformCfgChecks(ValidationState_t& _) {
<< _.getIdName(function.get_id());
}
// Updates each blocks immediate dominators
// Prepare for dominance calculations. We want to analyze all the
// blocks in the function, even in degenerate control flow cases
// including unreachable blocks. For this calculation, we create an
// agumented CFG by adding a pseudo-entry node that is considered the
// predecessor of each source in the original CFG, and a pseudo-exit
// node that is considered the successor to each sink in the original
// CFG. The augmented CFG is guaranteed to have a single source node
// (i.e. not having predecessors) and a single exit node (i.e. not
// having successors). However, there might be isolated strongly
// connected components that are not reachable by following successors
// from the pseudo entry node, and not reachable by following
// predecessors from the pseudo exit node.
auto* pseudo_entry = function.get_pseudo_entry_block();
auto* pseudo_exit = function.get_pseudo_exit_block();
// We need vectors to use as the predecessors (in the augmented CFG)
// for the source nodes of the original CFG. It must have a stable
// address for the duration of the calculation.
auto* pseudo_entry_vec = function.get_pseudo_entry_blocks();
// Similarly, we need a vector to be used as the successors (in the
// augmented CFG) for sinks in the original CFG.
auto* pseudo_exit_vec = function.get_pseudo_exit_blocks();
// Returns the predecessors of a block in the augmented CFG.
auto augmented_predecessor_fn = [pseudo_entry, pseudo_entry_vec](
const BasicBlock* block) {
auto predecessors = block->get_predecessors();
return (block != pseudo_entry && predecessors->empty()) ? pseudo_entry_vec
: predecessors;
};
// Returns the successors of a block in the augmented CFG.
auto augmented_successor_fn = [pseudo_exit,
pseudo_exit_vec](const BasicBlock* block) {
auto successors = block->get_successors();
return (block != pseudo_exit && successors->empty()) ? pseudo_exit_vec
: successors;
};
// Set each block's immediate dominator and immediate postdominator,
// and find all back-edges.
vector<const BasicBlock*> postorder;
vector<const BasicBlock*> postdom_postorder;
vector<pair<uint32_t, uint32_t>> back_edges;
if (auto* first_block = function.get_first_block()) {
if (!function.get_blocks().empty()) {
/// calculate dominators
DepthFirstTraversal(*first_block, successor, [](cbb_ptr) {},
DepthFirstTraversal(pseudo_entry, augmented_successor_fn, [](cbb_ptr) {},
[&](cbb_ptr b) { postorder.push_back(b); },
[&](cbb_ptr from, cbb_ptr to) {
back_edges.emplace_back(from->get_id(),
to->get_id());
});
auto edges = libspirv::CalculateDominators(postorder, predecessor);
libspirv::UpdateImmediateDominators(
edges, [](bb_ptr block, bb_ptr dominator) {
block->SetImmediateDominator(dominator);
});
auto edges =
libspirv::CalculateDominators(postorder, augmented_predecessor_fn);
for (auto edge : edges) {
edge.first->SetImmediateDominator(edge.second);
}
/// calculate post dominators
auto exit_block = function.get_pseudo_exit_block();
DepthFirstTraversal(*exit_block, predecessor, [](cbb_ptr) {},
DepthFirstTraversal(pseudo_exit, augmented_predecessor_fn, [](cbb_ptr) {},
[&](cbb_ptr b) { postdom_postorder.push_back(b); },
[&](cbb_ptr, cbb_ptr) {});
auto postdom_edges =
libspirv::CalculateDominators(postdom_postorder, successor);
libspirv::UpdateImmediateDominators(
postdom_edges, [](bb_ptr block, bb_ptr dominator) {
block->SetImmediatePostDominator(dominator);
});
auto postdom_edges = libspirv::CalculateDominators(
postdom_postorder, augmented_successor_fn);
for (auto edge : postdom_edges) {
edge.first->SetImmediatePostDominator(edge.second);
}
}
UpdateContinueConstructExitBlocks(function, back_edges);
@ -444,9 +464,10 @@ spv_result_t PerformCfgChecks(ValidationState_t& _) {
// dominate
auto& blocks = function.get_blocks();
if (blocks.empty() == false) {
for (auto block = begin(blocks) + 1; block != end(blocks); block++) {
for (auto block = begin(blocks) + 1; block != end(blocks); ++block) {
if (auto idom = (*block)->GetImmediateDominator()) {
if (block == std::find(begin(blocks), block, idom)) {
if (idom != pseudo_entry &&
block == std::find(begin(blocks), block, idom)) {
return _.diag(SPV_ERROR_INVALID_CFG)
<< "Block " << _.getIdName((*block)->get_id())
<< " appears in the binary before its dominator "

2
test/Validate.CFG.cpp
View File

@ -511,6 +511,7 @@ TEST_P(ValidateCFG, HeaderDoesntDominatesMergeBad) {
str += head >> vector<Block>({merge, f});
str += f >> merge;
str += merge;
str += "OpFunctionEnd\n";
CompileSuccessfully(str);
@ -679,6 +680,7 @@ TEST_P(ValidateCFG, NestedLoops) {
str += loop2_merge >> loop1;
str += loop1_merge >> exit;
str += exit;
str += "OpFunctionEnd";
CompileSuccessfully(str);
ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());

32
test/Validate.Layout.cpp
View File

@ -332,6 +332,38 @@ TEST_F(ValidateLayout, InstructionAppearBeforeFunctionDefinition) {
EXPECT_THAT(getDiagnosticString(), StrEq("Undef must appear in a block"));
}
TEST_F(ValidateLayout, MissingFunctionEndForFunctionWithBody) {
const auto s = R"(
OpCapability Shader
OpMemoryModel Logical GLSL450
%void = OpTypeVoid
%tf = OpTypeFunction %void
%f = OpFunction %void None %tf
%l = OpLabel
OpReturn
)";
CompileSuccessfully(s);
ASSERT_EQ(SPV_ERROR_INVALID_LAYOUT, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
StrEq("Missing OpFunctionEnd at end of module."));
}
TEST_F(ValidateLayout, MissingFunctionEndForFunctionPrototype) {
const auto s = R"(
OpCapability Shader
OpMemoryModel Logical GLSL450
%void = OpTypeVoid
%tf = OpTypeFunction %void
%f = OpFunction %void None %tf
)";
CompileSuccessfully(s);
ASSERT_EQ(SPV_ERROR_INVALID_LAYOUT, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
StrEq("Missing OpFunctionEnd at end of module."));
}
using ValidateOpFunctionParameter = spvtest::ValidateBase<int>;
TEST_F(ValidateOpFunctionParameter, OpLineBetweenParameters) {