Merge the verifier logic for all functions into a unified framework, this

requires enhancing DominanceInfo to handle the structure of an ML function,
which is required anyway.  Along the way, this also fixes a const correctness
problem with Instruction::getBlock().

This is step 24/n towards merging instructions and statements.

PiperOrigin-RevId: 227228900
This commit is contained in:
Chris Lattner 2018-12-29 09:11:58 -08:00 committed by jpienaar
parent 4a96a11d6d
commit 56e2a6cc3b
9 changed files with 252 additions and 269 deletions

View File

@ -65,9 +65,13 @@ public:
return (Instruction *)a->getDefiningInst() == b || properlyDominates(a, b);
}
// dominates/properlyDominates for blocks.
using DominatorTreeBase::dominates;
using DominatorTreeBase::properlyDominates;
/// Return true if the specified block A properly dominates block B.
bool properlyDominates(const Block *a, const Block *b);
/// Return true if the specified block A dominates block B.
bool dominates(const Block *a, const Block *b) {
return a == b || properlyDominates(a, b);
}
};
} // end namespace mlir

View File

@ -147,9 +147,11 @@ public:
/// Returns 'inst' if 'inst' lies in this block, or otherwise finds the
/// ancestor instruction of 'inst' that lies in this block. Returns nullptr if
/// the latter fails.
/// TODO: This is very specific functionality that should live somewhere else.
/// TODO: This is very specific functionality that should live somewhere else,
/// probably in Dominance.cpp.
const Instruction *findAncestorInstInBlock(const Instruction &inst) const;
/// TODO: This const overload is wrong.
// TODO: it doesn't make sense for the former method to take the instruction
// by reference but this one to take it by pointer.
Instruction *findAncestorInstInBlock(Instruction *inst) {
return const_cast<Instruction *>(findAncestorInstInBlock(*inst));
}

View File

@ -96,7 +96,8 @@ public:
Instruction *clone(MLIRContext *context) const;
/// Returns the instruction block that contains this instruction.
Block *getBlock() const { return block; }
const Block *getBlock() const { return block; }
Block *getBlock() { return block; }
/// Returns the closest surrounding instruction that contains this instruction
/// or nullptr if this is a top-level instruction.

View File

@ -919,10 +919,10 @@ static unsigned getNumCommonLoops(const FlatAffineConstraints &srcDomain,
}
// Returns Block common to 'srcAccess.opInst' and 'dstAccess.opInst'.
static Block *getCommonBlock(const MemRefAccess &srcAccess,
const MemRefAccess &dstAccess,
const FlatAffineConstraints &srcDomain,
unsigned numCommonLoops) {
static const Block *getCommonBlock(const MemRefAccess &srcAccess,
const MemRefAccess &dstAccess,
const FlatAffineConstraints &srcDomain,
unsigned numCommonLoops) {
if (numCommonLoops == 0) {
auto *block = srcAccess.opInst->getBlock();
while (block->getContainingInst()) {

View File

@ -35,35 +35,77 @@ DominanceInfo::DominanceInfo(Function *function) : DominatorTreeBase() {
recalculate(function->getBlockList());
}
/// Return true if instruction A properly dominates instruction B.
bool DominanceInfo::properlyDominates(const Instruction *a,
const Instruction *b) {
auto *aBlock = a->getBlock(), *bBlock = b->getBlock();
// If the blocks are different, it's as easy as whether A's block
// dominates B's block.
if (aBlock != bBlock)
return properlyDominates(a->getBlock(), b->getBlock());
// If a/b are the same, then they don't properly dominate each other.
bool DominanceInfo::properlyDominates(const Block *a, const Block *b) {
// A block dominates itself but does not properly dominate itself.
if (a == b)
return false;
// If one is a terminator, then the other dominates it.
if (a->isTerminator())
return false;
// If both blocks are in the same block list, then standard dominator
// information can resolve the query.
auto *blockListA = a->getParent(), *blockListB = b->getParent();
if (blockListA == blockListB)
return DominatorTreeBase::properlyDominates(a, b);
if (b->isTerminator())
return true;
// Otherwise, 'a' dominates 'b' if 'b' is defined in an IfInst/ForInst that
// (recursively) ends up being dominated by 'a'. Walk up the list of
// containers enclosing B.
while (blockListA != blockListB) {
// If 'b' is at a the top level function, then 'a' is defined inside some
// other instruction that doesn't dominate 'b'.
auto *containerInst = blockListB->getContainingInst();
if (!containerInst)
return false;
// Otherwise, do a linear scan to determine whether B comes after A.
auto aIter = Block::const_iterator(a);
auto bIter = Block::const_iterator(b);
auto fIter = aBlock->begin();
while (bIter != fIter) {
--bIter;
if (aIter == bIter)
blockListB = containerInst->getBlock()->getParent();
}
// Block 'A' is an ancestor of 'B', we know that A dominates B.
return true;
}
/// Return true if instruction A properly dominates instruction B.
bool DominanceInfo::properlyDominates(const Instruction *a,
const Instruction *b) {
auto *aBlock = a->getBlock();
auto *bBlock = b->getBlock();
// If the blocks are the same, then we do a linear scan.
if (aBlock == bBlock) {
// If a/b are the same, then they don't properly dominate each other.
if (a == b)
return false;
// If one is a terminator, then the other dominates it.
if (a->isTerminator())
return false;
if (b->isTerminator())
return true;
// Otherwise, do a linear scan to determine whether B comes after A.
// TODO: This is an O(n) scan that can be bad for very large blocks.
auto aIter = Block::const_iterator(a);
auto bIter = Block::const_iterator(b);
auto fIter = aBlock->begin();
while (bIter != fIter) {
--bIter;
if (aIter == bIter)
return true;
}
return false;
}
// If the blocks are different, but in the same function-level block list,
// then a standard block dominance query is sufficient.
if (aBlock->getParent()->getContainingFunction() &&
bBlock->getParent()->getContainingFunction())
return DominatorTreeBase::properlyDominates(aBlock, bBlock);
// Traverse up b's hierarchy to check if b's block is contained in a's.
if (auto *bAncestor = aBlock->findAncestorInstInBlock(*b)) {
// a and bAncestor are in the same block; check if 'a' dominates
// bAncestor.
return properlyDominates(a, bAncestor);
}
return false;
@ -74,7 +116,12 @@ bool DominanceInfo::properlyDominates(const Value *a, const Instruction *b) {
if (auto *aInst = a->getDefiningInst())
return properlyDominates(aInst, b);
// bbarguments properly dominate all instructions in their own block, so we
// use a dominates check here, not a properlyDominates check.
// The induction variable of a ForInst properly dominantes its body, so we
// can just do a simple block dominance check.
if (auto *forInst = dyn_cast<ForInst>(a))
return dominates(forInst->getBody(), b->getBlock());
// block arguments properly dominate all instructions in their own block, so
// we use a dominates check here, not a properlyDominates check.
return dominates(cast<BlockArgument>(a)->getOwner(), b->getBlock());
}

View File

@ -337,7 +337,7 @@ template bool mlir::boundCheckLoadOrStoreOp(OpPointer<StoreOp> storeOp,
// Block from the Block containing instruction, stopping at 'limitBlock'.
static void findInstPosition(const Instruction *inst, Block *limitBlock,
SmallVectorImpl<unsigned> *positions) {
Block *block = inst->getBlock();
const Block *block = inst->getBlock();
while (block != limitBlock) {
int instPosInBlock = block->findInstPositionInBlock(*inst);
assert(instPosInBlock >= 0);

View File

@ -36,20 +36,18 @@
#include "mlir/Analysis/Dominance.h"
#include "mlir/IR/Attributes.h"
#include "mlir/IR/Function.h"
#include "mlir/IR/InstVisitor.h"
#include "mlir/IR/Instructions.h"
#include "mlir/IR/Module.h"
#include "llvm/ADT/ScopedHashTable.h"
#include "llvm/Support/PrettyStackTrace.h"
#include "llvm/Support/raw_ostream.h"
using namespace mlir;
namespace {
/// Base class for the verifiers in this file. It is a pervasive truth that
/// this file treats "true" as an error that needs to be recovered from, and
/// "false" as success.
/// This class encapsulates all the state used to verify a function body. It is
/// a pervasive truth that this file treats "true" as an error that needs to be
/// recovered from, and "false" as success.
///
class Verifier {
class FuncVerifier {
public:
bool failure(const Twine &message, const OperationInst &value) {
return value.emitError(message);
@ -69,20 +67,90 @@ public:
return failure(message, fn);
}
bool verifyOperation(const OperationInst &op);
bool verifyAttribute(Attribute attr, const OperationInst &op);
protected:
explicit Verifier(const Function &fn) : fn(fn) {}
bool verify();
bool verifyBlock(const Block &block, bool isTopLevel);
bool verifyOperation(const OperationInst &op);
bool verifyForInst(const ForInst &forInst);
bool verifyIfInst(const IfInst &ifInst);
bool verifyDominance(const Block &block);
bool verifyInstDominance(const Instruction &inst);
explicit FuncVerifier(const Function &fn) : fn(fn) {}
private:
/// The function being checked.
const Function &fn;
/// Dominance information for this function, when checking dominance.
DominanceInfo *domInfo = nullptr;
};
} // end anonymous namespace
bool FuncVerifier::verify() {
llvm::PrettyStackTraceFormat fmt("MLIR Verifier: func @%s",
fn.getName().c_str());
// If this is an external function, it must be empty.
if (fn.getKind() == Function::Kind::ExtFunc) {
if (!fn.empty())
return failure("extfunc must not have any blocks", fn);
// nothing else to check.
return false;
}
if (fn.empty())
return failure("function must have at least one block", fn);
// ML Functions should have exactly one block.
// TODO(clattner): This will change real soon now.
if (fn.isML() && fn.getBlocks().size() != 1)
return fn.emitError("mlfunc should have exactly one block");
// Verify the first block has no predecessors.
auto *firstBB = &fn.front();
if (!firstBB->hasNoPredecessors())
return failure("entry block of function may not have predecessors", fn);
// Verify that the argument list of the function and the arg list of the first
// block line up.
auto fnInputTypes = fn.getType().getInputs();
if (fnInputTypes.size() != firstBB->getNumArguments())
return failure("first block of function must have " +
Twine(fnInputTypes.size()) +
" arguments to match function signature",
fn);
for (unsigned i = 0, e = firstBB->getNumArguments(); i != e; ++i)
if (fnInputTypes[i] != firstBB->getArgument(i)->getType())
return failure(
"type of argument #" + Twine(i) +
" must match corresponding argument in function signature",
fn);
for (auto &block : fn) {
if (verifyBlock(block, /*isTopLevel*/ true))
return true;
}
// Since everything looks structurally ok to this point, we do a dominance
// check. We do this as a second pass since malformed CFG's can cause
// dominator analysis constructure to crash and we want the verifier to be
// resilient to malformed code.
DominanceInfo theDomInfo(const_cast<Function *>(&fn));
domInfo = &theDomInfo;
for (auto &block : fn) {
if (verifyDominance(block))
return true;
}
domInfo = nullptr;
return false;
}
// Check that function attributes are all well formed.
bool Verifier::verifyAttribute(Attribute attr, const OperationInst &op) {
bool FuncVerifier::verifyAttribute(Attribute attr, const OperationInst &op) {
if (!attr.isOrContainsFunction())
return false;
@ -109,8 +177,38 @@ bool Verifier::verifyAttribute(Attribute attr, const OperationInst &op) {
return false;
}
bool FuncVerifier::verifyBlock(const Block &block, bool isTopLevel) {
// Blocks under IfInst/ForInst don't have terminators, but blocks at the top
// level of a function do.
if (isTopLevel && !block.getTerminator())
return failure("block with no terminator", block);
for (auto *arg : block.getArguments()) {
if (arg->getOwner() != &block)
return failure("block argument not owned by block", block);
}
for (auto &inst : block) {
switch (inst.getKind()) {
case Instruction::Kind::OperationInst:
if (verifyOperation(cast<OperationInst>(inst)))
return true;
break;
case Instruction::Kind::For:
if (verifyForInst(cast<ForInst>(inst)))
return true;
break;
case Instruction::Kind::If:
if (verifyIfInst(cast<IfInst>(inst)))
return true;
break;
}
}
return false;
}
/// Check the invariants of the specified operation.
bool Verifier::verifyOperation(const OperationInst &op) {
bool FuncVerifier::verifyOperation(const OperationInst &op) {
if (op.getFunction() != &fn)
return failure("operation in the wrong function", op);
@ -140,68 +238,57 @@ bool Verifier::verifyOperation(const OperationInst &op) {
return false;
}
//===----------------------------------------------------------------------===//
// CFG Functions
//===----------------------------------------------------------------------===//
bool FuncVerifier::verifyForInst(const ForInst &forInst) {
// TODO: check that loop bounds are properly formed.
return verifyBlock(*forInst.getBody(), /*isTopLevel=*/false);
}
namespace {
struct CFGFuncVerifier : public Verifier {
const Function &fn;
DominanceInfo domInfo;
bool FuncVerifier::verifyIfInst(const IfInst &ifInst) {
// TODO: check that if conditions are properly formed.
if (verifyBlock(*ifInst.getThen(), /*isTopLevel*/ false))
return true;
CFGFuncVerifier(const Function &fn)
: Verifier(fn), fn(fn), domInfo(const_cast<Function *>(&fn)) {}
bool verify();
bool verifyBlock(const Block &block);
bool verifyInstOperands(const Instruction &inst);
};
} // end anonymous namespace
bool CFGFuncVerifier::verify() {
llvm::PrettyStackTraceFormat fmt("MLIR Verifier: cfgfunc @%s",
fn.getName().c_str());
// TODO: Lots to be done here, including verifying dominance information when
// we have uses and defs.
if (fn.empty())
return failure("cfgfunc must have at least one block", fn);
// Verify the first block has no predecessors.
auto *firstBB = &fn.front();
if (!firstBB->hasNoPredecessors()) {
return failure("first block of cfgfunc must not have predecessors", fn);
}
// Verify that the argument list of the function and the arg list of the first
// block line up.
auto fnInputTypes = fn.getType().getInputs();
if (fnInputTypes.size() != firstBB->getNumArguments())
return failure("first block of cfgfunc must have " +
Twine(fnInputTypes.size()) +
" arguments to match function signature",
fn);
for (unsigned i = 0, e = firstBB->getNumArguments(); i != e; ++i)
if (fnInputTypes[i] != firstBB->getArgument(i)->getType())
return failure(
"type of argument #" + Twine(i) +
" must match corresponding argument in function signature",
fn);
for (auto &block : fn) {
if (verifyBlock(block))
if (auto *elseClause = ifInst.getElse())
if (verifyBlock(*elseClause, /*isTopLevel*/ false))
return true;
return false;
}
bool FuncVerifier::verifyDominance(const Block &block) {
for (auto &inst : block) {
// Check that all operands on the instruction are ok.
if (verifyInstDominance(inst))
return true;
switch (inst.getKind()) {
case Instruction::Kind::OperationInst:
if (verifyOperation(cast<OperationInst>(inst)))
return true;
break;
case Instruction::Kind::For:
if (verifyDominance(*cast<ForInst>(inst).getBody()))
return true;
break;
case Instruction::Kind::If:
auto &ifInst = cast<IfInst>(inst);
if (verifyDominance(*ifInst.getThen()))
return true;
if (auto *elseClause = ifInst.getElse())
if (verifyDominance(*elseClause))
return true;
break;
}
}
return false;
}
bool CFGFuncVerifier::verifyInstOperands(const Instruction &inst) {
bool FuncVerifier::verifyInstDominance(const Instruction &inst) {
// Check that operands properly dominate this use.
for (unsigned operandNo = 0, e = inst.getNumOperands(); operandNo != e;
++operandNo) {
auto *op = inst.getOperand(operandNo);
if (domInfo.properlyDominates(op, &inst))
if (domInfo->properlyDominates(op, &inst))
continue;
inst.emitError("operand #" + Twine(operandNo) +
@ -214,151 +301,6 @@ bool CFGFuncVerifier::verifyInstOperands(const Instruction &inst) {
return false;
}
bool CFGFuncVerifier::verifyBlock(const Block &block) {
if (!block.getTerminator())
return failure("block with no terminator", block);
for (auto *arg : block.getArguments()) {
if (arg->getOwner() != &block)
return failure("block argument not owned by block", block);
}
for (auto &inst : block) {
if (auto *opInst = dyn_cast<OperationInst>(&inst))
if (verifyOperation(*opInst))
return true;
if (verifyInstOperands(inst))
return true;
}
return false;
}
//===----------------------------------------------------------------------===//
// ML Functions
//===----------------------------------------------------------------------===//
namespace {
struct MLFuncVerifier : public Verifier, public InstWalker<MLFuncVerifier> {
const Function &fn;
bool hadError = false;
MLFuncVerifier(const Function &fn) : Verifier(fn), fn(fn) {}
void visitOperationInst(OperationInst *opInst) {
hadError |= verifyOperation(*opInst);
}
bool verify() {
llvm::PrettyStackTraceFormat fmt("MLIR Verifier: mlfunc @%s",
fn.getName().c_str());
// ML Functions should have exactly one block.
// TODO(clattner): This will change real soon now.
if (fn.getBlocks().size() != 1)
return fn.emitError("mlfunc should have exactly one block");
// Check basic structural properties.
walk(const_cast<Function *>(&fn));
if (hadError)
return true;
// TODO: check that loop bounds and if conditions are properly formed.
if (verifyReturn())
return true;
return verifyDominance();
}
/// Walk all of the code in this MLFunc and verify that the operands of any
/// operations are properly dominated by their definitions.
bool verifyDominance();
/// Verify that function has a return instruction that matches its signature.
bool verifyReturn();
};
} // end anonymous namespace
/// Walk all of the code in this MLFunc and verify that the operands of any
/// operations are properly dominated by their definitions.
bool MLFuncVerifier::verifyDominance() {
using HashTable = llvm::ScopedHashTable<const Value *, bool>;
HashTable liveValues;
HashTable::ScopeTy topScope(liveValues);
// All of the arguments to the function are live for the whole function.
for (auto *arg : fn.getArguments())
liveValues.insert(arg, true);
// This recursive function walks the instruction list pushing scopes onto the
// stack as it goes, and popping them to remove them from the table.
std::function<bool(const Block &block)> walkBlock;
walkBlock = [&](const Block &block) -> bool {
HashTable::ScopeTy blockScope(liveValues);
// The induction variable of a for instruction is live within its body.
if (auto *forInst = dyn_cast_or_null<ForInst>(block.getContainingInst()))
liveValues.insert(forInst, true);
for (auto &inst : block) {
// Verify that each of the operands are live.
unsigned operandNo = 0;
for (auto *opValue : inst.getOperands()) {
if (!liveValues.count(opValue)) {
inst.emitError("operand #" + Twine(operandNo) +
" does not dominate this use");
if (auto *useInst = opValue->getDefiningInst())
useInst->emitNote("operand defined here");
return true;
}
++operandNo;
}
if (auto *opInst = dyn_cast<OperationInst>(&inst)) {
// Operations define values, add them to the hash table.
for (auto *result : opInst->getResults())
liveValues.insert(result, true);
continue;
}
// If this is an if or for, recursively walk the block they contain.
if (auto *ifInst = dyn_cast<IfInst>(&inst)) {
if (walkBlock(*ifInst->getThen()))
return true;
if (auto *elseClause = ifInst->getElse())
if (walkBlock(*elseClause))
return true;
}
if (auto *forInst = dyn_cast<ForInst>(&inst))
if (walkBlock(*forInst->getBody()))
return true;
}
return false;
};
// Check the whole function out.
return walkBlock(*fn.getBody());
}
bool MLFuncVerifier::verifyReturn() {
// TODO: fold return verification in the pass that verifies all instructions.
const char missingReturnMsg[] =
"ML function must end with return instruction";
if (fn.getBody()->getInstructions().empty())
return failure(missingReturnMsg, fn);
const auto &inst = fn.getBody()->getInstructions().back();
if (const auto *op = dyn_cast<OperationInst>(&inst)) {
if (!op->isReturn())
return failure(missingReturnMsg, fn);
return false;
}
return failure(missingReturnMsg, fn);
}
//===----------------------------------------------------------------------===//
// Entrypoints
//===----------------------------------------------------------------------===//
@ -366,17 +308,7 @@ bool MLFuncVerifier::verifyReturn() {
/// Perform (potentially expensive) checks of invariants, used to detect
/// compiler bugs. On error, this reports the error through the MLIRContext and
/// returns true.
bool Function::verify() const {
switch (getKind()) {
case Kind::ExtFunc:
// No body, nothing can be wrong here.
return false;
case Kind::CFGFunc:
return CFGFuncVerifier(*this).verify();
case Kind::MLFunc:
return MLFuncVerifier(*this).verify();
}
}
bool Function::verify() const { return FuncVerifier(*this).verify(); }
/// Perform (potentially expensive) checks of invariants, used to detect
/// compiler bugs. On error, this reports the error through the MLIRContext and

View File

@ -275,17 +275,14 @@ static bool verifyTerminatorSuccessors(const OperationInst *op) {
}
bool OpTrait::impl::verifyIsTerminator(const OperationInst *op) {
const Block *block = op->getBlock();
// Verify that the operation is at the end of the respective parent block.
if (op->getFunction()->isML()) {
Block *block = op->getBlock();
if (!block || block->getContainingInst() || &block->back() != op)
return op->emitOpError("must be the last instruction in the ML function");
} else {
const Block *block = op->getBlock();
if (!block || &block->back() != op)
return op->emitOpError(
"must be the last instruction in the parent block");
}
if (!block || &block->back() != op)
return op->emitOpError("must be the last instruction in the parent block");
// Terminators may not exist in ForInst and IfInst.
if (block->getContainingInst())
return op->emitOpError("may only be at the top level of a function");
// Verify the state of the successor blocks.
if (op->getNumSuccessors() != 0 && verifyTerminatorSuccessors(op))

View File

@ -142,7 +142,7 @@ bb42:
// -----
cfgfunc @block_first_has_predecessor() {
// expected-error@-1 {{first block of cfgfunc must not have predecessors}}
// expected-error@-1 {{entry block of function may not have predecessors}}
bb42:
br bb43
bb43:
@ -177,8 +177,8 @@ bb42:
// -----
mlfunc @no_return() { // expected-error {{ML function must end with return instruction}}
"foo"() : () -> ()
mlfunc @no_return() {
"foo"() : () -> () // expected-error {{block with no terminator}}
}
// -----
@ -370,7 +370,7 @@ bb2(%a: i64): // expected-error{{redefinition of SSA value '%a'}}
// -----
cfgfunc @bbargMismatch(i32, f32) { // expected-error {{first block of cfgfunc must have 2 arguments to match function signature}}
cfgfunc @bbargMismatch(i32, f32) { // expected-error {{first block of function must have 2 arguments to match function signature}}
bb42(%0: f32):
return
}
@ -478,7 +478,7 @@ mlfunc @return_inside_loop() -> i8 {
for %i = 1 to 100 {
%a = "foo"() : ()->i8
return %a : i8
// expected-error@-1 {{'return' op must be the last instruction in the ML function}}
// expected-error@-1 {{'return' op may only be at the top level of a function}}
}
}