maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[Verifier] Tidy up the code a bit, NFC. 

This tidies up the code a bit:
 * Eliminate the ctx member, which doesn't need to be stored.
 * Rename verify(Operation) to make it more clear that it is
   doing more than verifyOperation and that the dominance check
   isn't being done multiple times.
 * Rename mayNotHaveTerminator which was confusing about whether
   it wasn't known whether it had a terminator, when it is really
   about whether it is legal to have a terminator.
 * Some minor optimizations: don't check for RegionKindInterface
   if there are no regions.  Don't do two passes over the
   operations in a block in OperationVerifier::verifyDominance when
   one will do.

The optimizations are actually a measurable (but minor) win in some
CIRCT cases.

Differential Revision: https://reviews.llvm.org/D101267
This commit is contained in:
Chris Lattner 2021-04-25 16:15:17 -07:00
parent abd860eaef
commit f0c22c3d58
1 changed files with 60 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

116
mlir/lib/IR/Verifier.cpp
View File

@ -41,10 +41,10 @@ namespace {
/// This class encapsulates all the state used to verify an operation region. /// This class encapsulates all the state used to verify an operation region.
class OperationVerifier { class OperationVerifier {
public: public:
explicit OperationVerifier(MLIRContext *ctx) : ctx(ctx) {} explicit OperationVerifier() {}
/// Verify the given operation. /// Verify the given operation.
LogicalResult verify(Operation &op); LogicalResult verifyOpAndDominance(Operation &op);
private: private:
/// Verify the given potentially nested region or block. /// Verify the given potentially nested region or block.
@ -69,16 +69,13 @@ private:
return mlir::emitError(bb.getParent()->getLoc(), message); return mlir::emitError(bb.getParent()->getLoc(), message);
} }
/// The current context for the verifier.
MLIRContext *ctx;
/// Dominance information for this operation, when checking dominance. /// Dominance information for this operation, when checking dominance.
DominanceInfo *domInfo = nullptr; DominanceInfo *domInfo = nullptr;
}; };
} // end anonymous namespace } // end anonymous namespace
/// Verify the given operation. /// Verify the given operation.
LogicalResult OperationVerifier::verify(Operation &op) { LogicalResult OperationVerifier::verifyOpAndDominance(Operation &op) {
// Verify the operation first. // Verify the operation first.
if (failed(verifyOperation(op))) if (failed(verifyOperation(op)))
return failure(); return failure();
@ -119,7 +116,7 @@ LogicalResult OperationVerifier::verifyRegion(Region &region) {
/// - This region does not have a parent op. /// - This region does not have a parent op.
/// - Or the parent op is unregistered. /// - Or the parent op is unregistered.
/// - Or the parent op has the NoTerminator trait. /// - Or the parent op has the NoTerminator trait.
static bool mayNotHaveTerminator(Block *block) { static bool mayBeValidWithoutTerminator(Block *block) {
if (!block->getParent()) if (!block->getParent())
return true; return true;
if (!llvm::hasSingleElement(*block->getParent())) if (!llvm::hasSingleElement(*block->getParent()))
@ -134,9 +131,8 @@ LogicalResult OperationVerifier::verifyBlock(Block &block) {
return emitError(block, "block argument not owned by block"); return emitError(block, "block argument not owned by block");
// Verify that this block has a terminator. // Verify that this block has a terminator.
if (block.empty()) { if (block.empty()) {
if (mayNotHaveTerminator(&block)) if (mayBeValidWithoutTerminator(&block))
return success(); return success();
return emitError(block, "empty block: expect at least a terminator"); return emitError(block, "empty block: expect at least a terminator");
} }
@ -157,13 +153,6 @@ LogicalResult OperationVerifier::verifyBlock(Block &block) {
if (failed(verifyOperation(terminator))) if (failed(verifyOperation(terminator)))
return failure(); return failure();
if (mayNotHaveTerminator(&block))
return success();
if (!terminator.mightHaveTrait<OpTrait::IsTerminator>())
return block.back().emitError("block with no terminator, has ")
<< terminator;
// Verify that this block is not branching to a block of a different // Verify that this block is not branching to a block of a different
// region. // region.
for (Block *successor : block.getSuccessors()) for (Block *successor : block.getSuccessors())
@ -171,6 +160,14 @@ LogicalResult OperationVerifier::verifyBlock(Block &block) {
return block.back().emitOpError( return block.back().emitOpError(
"branching to block of a different region"); "branching to block of a different region");
// If this block doesn't have to have a terminator, don't require it.
if (mayBeValidWithoutTerminator(&block))
return success();
if (!terminator.mightHaveTrait<OpTrait::IsTerminator>())
return block.back().emitError("block with no terminator, has ")
<< terminator;
return success(); return success();
} }
@ -194,31 +191,32 @@ LogicalResult OperationVerifier::verifyOperation(Operation &op) {
if (opInfo && failed(opInfo->verifyInvariants(&op))) if (opInfo && failed(opInfo->verifyInvariants(&op)))
return failure(); return failure();
auto kindInterface = dyn_cast<mlir::RegionKindInterface>(op); if (unsigned numRegions = op.getNumRegions()) {
auto kindInterface = dyn_cast<mlir::RegionKindInterface>(op);
// Verify that all child regions are ok. // Verify that all child regions are ok.
unsigned numRegions = op.getNumRegions(); for (unsigned i = 0; i < numRegions; ++i) {
for (unsigned i = 0; i < numRegions; i++) { Region &region = op.getRegion(i);
Region &region = op.getRegion(i); RegionKind kind =
RegionKind kind = kindInterface ? kindInterface.getRegionKind(i) : RegionKind::SSACFG;
kindInterface ? kindInterface.getRegionKind(i) : RegionKind::SSACFG; // Check that Graph Regions only have a single basic block. This is
// Check that Graph Regions only have a single basic block. This is // similar to the code in SingleBlockImplicitTerminator, but doesn't
// similar to the code in SingleBlockImplicitTerminator, but doesn't // require the trait to be specified. This arbitrary limitation is
// require the trait to be specified. This arbitrary limitation is // designed to limit the number of cases that have to be handled by
// designed to limit the number of cases that have to be handled by // transforms and conversions.
// transforms and conversions until the concept stabilizes. if (op.isRegistered() && kind == RegionKind::Graph) {
if (op.isRegistered() && kind == RegionKind::Graph) { // Empty regions are fine.
// Empty regions are fine. if (region.empty())
if (region.empty()) continue;
continue;
// Non-empty regions must contain a single basic block. // Non-empty regions must contain a single basic block.
if (std::next(region.begin()) != region.end()) if (std::next(region.begin()) != region.end())
return op.emitOpError("expects graph region #") return op.emitOpError("expects graph region #")
<< i << " to have 0 or 1 blocks"; << i << " to have 0 or 1 blocks";
}
if (failed(verifyRegion(region)))
return failure();
} }
if (failed(verifyRegion(region)))
return failure();
} }
// If this is a registered operation, there is nothing left to do. // If this is a registered operation, there is nothing left to do.
@ -228,7 +226,7 @@ LogicalResult OperationVerifier::verifyOperation(Operation &op) {
// Otherwise, verify that the parent dialect allows un-registered operations. // Otherwise, verify that the parent dialect allows un-registered operations.
Dialect *dialect = opName.getDialect(); Dialect *dialect = opName.getDialect();
if (!dialect) { if (!dialect) {
if (!ctx->allowsUnregisteredDialects()) { if (!op.getContext()->allowsUnregisteredDialects()) {
return op.emitOpError() return op.emitOpError()
<< "created with unregistered dialect. If this is " << "created with unregistered dialect. If this is "
"intended, please call allowUnregisteredDialects() on the " "intended, please call allowUnregisteredDialects() on the "
@ -247,10 +245,16 @@ LogicalResult OperationVerifier::verifyOperation(Operation &op) {
return success(); return success();
} }
/// Attach a note to an in-flight diagnostic that provide more information about /// Emit an error when the specified operand of the specified operation is an
/// where an op operand is defined. /// invalid use because of dominance properties.
static void attachNoteForOperandDefinition(InFlightDiagnostic &diag, static void diagnoseInvalidOperandDominance(Operation &op, unsigned operandNo) {
Operation &op, Value operand) { InFlightDiagnostic diag = op.emitError("operand #")
<< operandNo << " does not dominate this use";
Value operand = op.getOperand(operandNo);
/// Attach a note to an in-flight diagnostic that provide more information
/// about where an op operand is defined.
if (auto *useOp = operand.getDefiningOp()) { if (auto *useOp = operand.getDefiningOp()) {
Diagnostic &note = diag.attachNote(useOp->getLoc()); Diagnostic &note = diag.attachNote(useOp->getLoc());
note << "operand defined here"; note << "operand defined here";
@ -301,27 +305,27 @@ LogicalResult OperationVerifier::verifyDominance(Region &region) {
// Verify the dominance of each of the held operations. // Verify the dominance of each of the held operations.
for (Block &block : region) { for (Block &block : region) {
// Dominance is only meaningful inside reachable blocks. // Dominance is only meaningful inside reachable blocks.
if (domInfo->isReachableFromEntry(&block)) bool isReachable = domInfo->isReachableFromEntry(&block);
for (Operation &op : block)
for (Operation &op : block) {
if (isReachable) {
// Check that operands properly dominate this use. // Check that operands properly dominate this use.
for (unsigned operandNo = 0, e = op.getNumOperands(); operandNo != e; for (unsigned operandNo = 0, e = op.getNumOperands(); operandNo != e;
++operandNo) { ++operandNo) {
Value operand = op.getOperand(operandNo); if (domInfo->properlyDominates(op.getOperand(operandNo), &op))
if (domInfo->properlyDominates(operand, &op))
continue; continue;
InFlightDiagnostic diag = op.emitError("operand #") diagnoseInvalidOperandDominance(op, operandNo);
<< operandNo
<< " does not dominate this use";
attachNoteForOperandDefinition(diag, op, operand);
return failure(); return failure();
} }
// Recursively verify dominance within each operation in the }
// block, even if the block itself is not reachable, or we are in
// a region which doesn't respect dominance. // Recursively verify dominance within each operation in the
for (Operation &op : block) // block, even if the block itself is not reachable, or we are in
// a region which doesn't respect dominance.
if (failed(verifyDominanceOfContainedRegions(op))) if (failed(verifyDominanceOfContainedRegions(op)))
return failure(); return failure();
}
} }
return success(); return success();
} }
@ -344,5 +348,5 @@ OperationVerifier::verifyDominanceOfContainedRegions(Operation &op) {
/// compiler bugs. On error, this reports the error through the MLIRContext and /// compiler bugs. On error, this reports the error through the MLIRContext and
/// returns failure. /// returns failure.
LogicalResult mlir::verify(Operation *op) { LogicalResult mlir::verify(Operation *op) {
return OperationVerifier(op->getContext()).verify(*op); return OperationVerifier().verifyOpAndDominance(*op);
} }