[Mlir-commits] [mlir] ad6a84f - Revert "[Verifier] Speed up and parallelize dominance checking. NFC"
Alexander Kornienko
llvmlistbot at llvm.org
Thu Jun 10 01:01:03 PDT 2021
Author: Alexander Kornienko
Date: 2021-06-10T09:58:05+02:00
New Revision: ad6a84f82c4572dd92369b4f67df6e7c3536f9a2
URL: https://github.com/llvm/llvm-project/commit/ad6a84f82c4572dd92369b4f67df6e7c3536f9a2
DIFF: https://github.com/llvm/llvm-project/commit/ad6a84f82c4572dd92369b4f67df6e7c3536f9a2.diff
LOG: Revert "[Verifier] Speed up and parallelize dominance checking. NFC"
This reverts commit 08664d005c02003180371049b19c7e5d01541c58, which according to
https://reviews.llvm.org/D103373 was pushed accidentally, and I believe it
causes timeouts in some internal mlir tests.
Added:
Modified:
mlir/include/mlir/IR/Dominance.h
mlir/lib/IR/Verifier.cpp
Removed:
################################################################################
diff --git a/mlir/include/mlir/IR/Dominance.h b/mlir/include/mlir/IR/Dominance.h
index dcf45130ad1ea..68513c27aa0ea 100644
--- a/mlir/include/mlir/IR/Dominance.h
+++ b/mlir/include/mlir/IR/Dominance.h
@@ -35,7 +35,7 @@ class DominanceInfoBase {
using DomTree = llvm::DominatorTreeBase<Block, IsPostDom>;
public:
- DominanceInfoBase(Operation *op = nullptr) {}
+ DominanceInfoBase(Operation *op) {}
DominanceInfoBase(DominanceInfoBase &&) = default;
DominanceInfoBase &operator=(DominanceInfoBase &&) = default;
~DominanceInfoBase();
diff --git a/mlir/lib/IR/Verifier.cpp b/mlir/lib/IR/Verifier.cpp
index ffb8236ffc701..ec1ee8b2db241 100644
--- a/mlir/lib/IR/Verifier.cpp
+++ b/mlir/lib/IR/Verifier.cpp
@@ -31,8 +31,9 @@
#include "mlir/IR/Operation.h"
#include "mlir/IR/RegionKindInterface.h"
#include "llvm/ADT/StringMap.h"
-#include "llvm/Support/Parallel.h"
+#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Regex.h"
#include <atomic>
@@ -42,8 +43,7 @@ namespace {
/// This class encapsulates all the state used to verify an operation region.
class OperationVerifier {
public:
- explicit OperationVerifier(MLIRContext *context)
- : parallelismEnabled(context->isMultithreadingEnabled()) {}
+ explicit OperationVerifier() {}
/// Verify the given operation.
LogicalResult verifyOpAndDominance(Operation &op);
@@ -56,8 +56,7 @@ class OperationVerifier {
/// Verify the dominance property of regions contained within the given
/// Operation.
- LogicalResult verifyDominanceOfContainedRegions(Operation &op,
- DominanceInfo &domInfo);
+ LogicalResult verifyDominanceOfContainedRegions(Operation &op);
/// Emit an error for the given block.
InFlightDiagnostic emitError(Block &bb, const Twine &message) {
@@ -69,8 +68,8 @@ class OperationVerifier {
return mlir::emitError(bb.getParent()->getLoc(), message);
}
- /// This is true if parallelism is enabled on the MLIRContext.
- const bool parallelismEnabled;
+ /// Dominance information for this operation, when checking dominance.
+ DominanceInfo *domInfo = nullptr;
};
} // end anonymous namespace
@@ -84,11 +83,12 @@ LogicalResult OperationVerifier::verifyOpAndDominance(Operation &op) {
// check for any nested regions. 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.
- if (op.getNumRegions() != 0) {
- DominanceInfo domInfo;
- if (failed(verifyDominanceOfContainedRegions(op, /*domInfo*/ domInfo)))
- return failure();
- }
+ DominanceInfo theDomInfo(&op);
+ domInfo = &theDomInfo;
+ if (failed(verifyDominanceOfContainedRegions(op)))
+ return failure();
+
+ domInfo = nullptr;
return success();
}
@@ -301,83 +301,34 @@ static void diagnoseInvalidOperandDominance(Operation &op, unsigned operandNo) {
note << " neither in a parent nor in a child region)";
}
-/// Verify the dominance of each of the nested blocks within the given
-/// operation. domInfo may be present or absent (null), depending on whether
-/// the caller computed it for a higher level.
+/// Verify the dominance of each of the nested blocks within the given operation
LogicalResult
-OperationVerifier::verifyDominanceOfContainedRegions(Operation &opWithRegions,
- DominanceInfo &domInfo) {
- // This vector keeps track of ops that have regions which should be checked
- // in parallel.
- SmallVector<Operation *> opsWithRegionsToCheckInParallel;
-
- // Get information about the requirements on the regions in this op.
- for (Region ®ion : opWithRegions.getRegions()) {
+OperationVerifier::verifyDominanceOfContainedRegions(Operation &op) {
+ for (Region ®ion : op.getRegions()) {
+ // Verify the dominance of each of the held operations.
for (Block &block : region) {
// Dominance is only meaningful inside reachable blocks.
- bool isReachable = domInfo.isReachableFromEntry(&block);
-
- // Check each operation in this block, and any operations in regions
- // that these operations contain.
- opsWithRegionsToCheckInParallel.clear();
+ bool isReachable = domInfo->isReachableFromEntry(&block);
for (Operation &op : block) {
if (isReachable) {
// Check that operands properly dominate this use.
- for (auto &operand : op.getOpOperands()) {
- // If the operand doesn't dominate the user, then emit an error.
- if (!domInfo.properlyDominates(operand.get(), &op)) {
- diagnoseInvalidOperandDominance(op, operand.getOperandNumber());
- return failure();
- }
- }
- }
+ for (unsigned operandNo = 0, e = op.getNumOperands(); operandNo != e;
+ ++operandNo) {
+ if (domInfo->properlyDominates(op.getOperand(operandNo), &op))
+ continue;
- // If this operation has any regions, we need to recursively verify
- // dominance of the ops within it.
- if (op.getNumRegions() == 0)
- continue;
-
- // If this is a non-isolated region (e.g. an affine for loop), pass down
- // the current dominator information.
- if (!op.hasTrait<OpTrait::IsIsolatedFromAbove>()) {
- if (failed(verifyDominanceOfContainedRegions(op, domInfo)))
- return failure();
- } else if (parallelismEnabled) {
- // If this is an IsolatedFromAbove op and parallelism is enabled, then
- // enqueue this for processing later.
- opsWithRegionsToCheckInParallel.push_back(&op);
- } else {
- // If not, just verify inline with a local dom scope.
- DominanceInfo localDomInfo;
- if (failed(verifyDominanceOfContainedRegions(op, localDomInfo)))
+ diagnoseInvalidOperandDominance(op, operandNo);
return failure();
+ }
}
- }
- // If we have multiple parallelizable subregions, check them in parallel.
- if (opsWithRegionsToCheckInParallel.size() == 1) {
- // Each isolated operation gets its own dom info.
- Operation *op = opsWithRegionsToCheckInParallel[0];
- DominanceInfo localDomInfo;
- if (failed(verifyDominanceOfContainedRegions(*op, localDomInfo)))
- return failure();
- } else if (!opsWithRegionsToCheckInParallel.empty()) {
- ParallelDiagnosticHandler handler(opWithRegions.getContext());
- std::atomic<bool> passFailed(false);
- llvm::parallelForEachN(
- 0, opsWithRegionsToCheckInParallel.size(), [&](size_t opIdx) {
- handler.setOrderIDForThread(opIdx);
- Operation *op = opsWithRegionsToCheckInParallel[opIdx];
-
- // Each isolated operation gets its own dom info.
- DominanceInfo localDomInfo;
- if (failed(verifyDominanceOfContainedRegions(*op, localDomInfo)))
- passFailed = true;
- handler.eraseOrderIDForThread();
- });
- if (passFailed)
- 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.
+ if (op.getNumRegions() != 0)
+ if (failed(verifyDominanceOfContainedRegions(op)))
+ return failure();
}
}
}
@@ -389,8 +340,8 @@ OperationVerifier::verifyDominanceOfContainedRegions(Operation &opWithRegions,
//===----------------------------------------------------------------------===//
/// Perform (potentially expensive) checks of invariants, used to detect
-/// compiler bugs. On error, this reports the error through the MLIRContext
-/// and returns failure.
+/// compiler bugs. On error, this reports the error through the MLIRContext and
+/// returns failure.
LogicalResult mlir::verify(Operation *op) {
- return OperationVerifier(op->getContext()).verifyOpAndDominance(*op);
+ return OperationVerifier().verifyOpAndDominance(*op);
}
More information about the Mlir-commits
mailing list