[Mlir-commits] [mlir] 008b9d9 - Make the implicit nesting behavior of the PassManager user-controllable and default to false
Mehdi Amini
llvmlistbot at llvm.org
Tue Nov 3 03:18:00 PST 2020
Author: Mehdi Amini
Date: 2020-11-03T11:17:44Z
New Revision: 008b9d97cb791b8e3d89c7bc3ddd714ca3a61c80
URL: https://github.com/llvm/llvm-project/commit/008b9d97cb791b8e3d89c7bc3ddd714ca3a61c80
DIFF: https://github.com/llvm/llvm-project/commit/008b9d97cb791b8e3d89c7bc3ddd714ca3a61c80.diff
LOG: Make the implicit nesting behavior of the PassManager user-controllable and default to false
This is an error prone behavior, I frequently have ~20 min debugging sessions when I hit
an unexpected implicit nesting. This default makes the C++ API safer for users.
Depends On D90669
Reviewed By: rriddle
Differential Revision: https://reviews.llvm.org/D90671
Added:
Modified:
mlir/examples/toy/Ch5/toyc.cpp
mlir/examples/toy/Ch6/toyc.cpp
mlir/examples/toy/Ch7/toyc.cpp
mlir/include/mlir/Pass/PassManager.h
mlir/lib/Pass/Pass.cpp
mlir/lib/Support/MlirOptMain.cpp
mlir/test/lib/Transforms/TestConvVectorization.cpp
mlir/test/lib/Transforms/TestDynamicPipeline.cpp
mlir/unittests/Pass/PassManagerTest.cpp
Removed:
################################################################################
diff --git a/mlir/examples/toy/Ch5/toyc.cpp b/mlir/examples/toy/Ch5/toyc.cpp
index 94c3bd573cdd..fd5fdc1abb63 100644
--- a/mlir/examples/toy/Ch5/toyc.cpp
+++ b/mlir/examples/toy/Ch5/toyc.cpp
@@ -136,10 +136,10 @@ int dumpMLIR() {
}
if (isLoweringToAffine) {
- // Partially lower the toy dialect with a few cleanups afterwards.
- pm.addPass(mlir::toy::createLowerToAffinePass());
-
mlir::OpPassManager &optPM = pm.nest<mlir::FuncOp>();
+
+ // Partially lower the toy dialect with a few cleanups afterwards.
+ optPM.addPass(mlir::toy::createLowerToAffinePass());
optPM.addPass(mlir::createCanonicalizerPass());
optPM.addPass(mlir::createCSEPass());
diff --git a/mlir/examples/toy/Ch6/toyc.cpp b/mlir/examples/toy/Ch6/toyc.cpp
index d597a1f987b0..c52478ed5244 100644
--- a/mlir/examples/toy/Ch6/toyc.cpp
+++ b/mlir/examples/toy/Ch6/toyc.cpp
@@ -150,10 +150,10 @@ int loadAndProcessMLIR(mlir::MLIRContext &context,
}
if (isLoweringToAffine) {
- // Partially lower the toy dialect with a few cleanups afterwards.
- pm.addPass(mlir::toy::createLowerToAffinePass());
-
mlir::OpPassManager &optPM = pm.nest<mlir::FuncOp>();
+
+ // Partially lower the toy dialect with a few cleanups afterwards.
+ optPM.addPass(mlir::toy::createLowerToAffinePass());
optPM.addPass(mlir::createCanonicalizerPass());
optPM.addPass(mlir::createCSEPass());
diff --git a/mlir/examples/toy/Ch7/toyc.cpp b/mlir/examples/toy/Ch7/toyc.cpp
index c28e2a8424dc..5928f800cd95 100644
--- a/mlir/examples/toy/Ch7/toyc.cpp
+++ b/mlir/examples/toy/Ch7/toyc.cpp
@@ -151,10 +151,10 @@ int loadAndProcessMLIR(mlir::MLIRContext &context,
}
if (isLoweringToAffine) {
- // Partially lower the toy dialect with a few cleanups afterwards.
- pm.addPass(mlir::toy::createLowerToAffinePass());
-
mlir::OpPassManager &optPM = pm.nest<mlir::FuncOp>();
+
+ // Partially lower the toy dialect with a few cleanups afterwards.
+ optPM.addPass(mlir::toy::createLowerToAffinePass());
optPM.addPass(mlir::createCanonicalizerPass());
optPM.addPass(mlir::createCSEPass());
diff --git a/mlir/include/mlir/Pass/PassManager.h b/mlir/include/mlir/Pass/PassManager.h
index 17bba93ee9a6..3080fc35a153 100644
--- a/mlir/include/mlir/Pass/PassManager.h
+++ b/mlir/include/mlir/Pass/PassManager.h
@@ -48,8 +48,9 @@ struct PassExecutionState;
/// other OpPassManagers or the top-level PassManager.
class OpPassManager {
public:
- OpPassManager(Identifier name);
- OpPassManager(StringRef name);
+ enum class Nesting { Implicit, Explicit };
+ OpPassManager(Identifier name, Nesting nesting);
+ OpPassManager(StringRef name, Nesting nesting);
OpPassManager(OpPassManager &&rhs);
OpPassManager(const OpPassManager &rhs);
~OpPassManager();
@@ -149,7 +150,7 @@ enum class PassDisplayMode {
/// The main pass manager and pipeline builder.
class PassManager : public OpPassManager {
public:
- PassManager(MLIRContext *ctx);
+ PassManager(MLIRContext *ctx, Nesting nesting = Nesting::Explicit);
~PassManager();
/// Run the passes within this manager on the provided module.
diff --git a/mlir/lib/Pass/Pass.cpp b/mlir/lib/Pass/Pass.cpp
index fb56d85e896b..610786574ff5 100644
--- a/mlir/lib/Pass/Pass.cpp
+++ b/mlir/lib/Pass/Pass.cpp
@@ -91,9 +91,10 @@ void VerifierPass::runOnOperation() {
namespace mlir {
namespace detail {
struct OpPassManagerImpl {
- OpPassManagerImpl(Identifier identifier)
- : name(identifier), identifier(identifier) {}
- OpPassManagerImpl(StringRef name) : name(name) {}
+ OpPassManagerImpl(Identifier identifier, OpPassManager::Nesting nesting)
+ : name(identifier), identifier(identifier), nesting(nesting) {}
+ OpPassManagerImpl(StringRef name, OpPassManager::Nesting nesting)
+ : name(name), nesting(nesting) {}
/// Merge the passes of this pass manager into the one provided.
void mergeInto(OpPassManagerImpl &rhs);
@@ -130,6 +131,10 @@ struct OpPassManagerImpl {
/// The set of passes to run as part of this pass manager.
std::vector<std::unique_ptr<Pass>> passes;
+
+ /// Control the implicit nesting of passes that mismatch the name set for this
+ /// OpPassManager.
+ OpPassManager::Nesting nesting;
};
} // end namespace detail
} // end namespace mlir
@@ -142,14 +147,14 @@ void OpPassManagerImpl::mergeInto(OpPassManagerImpl &rhs) {
}
OpPassManager &OpPassManagerImpl::nest(Identifier nestedName) {
- OpPassManager nested(nestedName);
+ OpPassManager nested(nestedName, nesting);
auto *adaptor = new OpToOpPassAdaptor(std::move(nested));
addPass(std::unique_ptr<Pass>(adaptor));
return adaptor->getPassManagers().front();
}
OpPassManager &OpPassManagerImpl::nest(StringRef nestedName) {
- OpPassManager nested(nestedName);
+ OpPassManager nested(nestedName, nesting);
auto *adaptor = new OpToOpPassAdaptor(std::move(nested));
addPass(std::unique_ptr<Pass>(adaptor));
return adaptor->getPassManagers().front();
@@ -157,10 +162,16 @@ OpPassManager &OpPassManagerImpl::nest(StringRef nestedName) {
void OpPassManagerImpl::addPass(std::unique_ptr<Pass> pass) {
// If this pass runs on a
diff erent operation than this pass manager, then
- // implicitly nest a pass manager for this operation.
+ // implicitly nest a pass manager for this operation if enabled.
auto passOpName = pass->getOpName();
- if (passOpName && passOpName != name)
- return nest(*passOpName).addPass(std::move(pass));
+ if (passOpName && passOpName != name) {
+ if (nesting == OpPassManager::Nesting::Implicit)
+ return nest(*passOpName).addPass(std::move(pass));
+ llvm::report_fatal_error(llvm::Twine("Can't add pass '") + pass->getName() +
+ "' restricted to '" + *passOpName +
+ "' on a PassManager intended to run on '" + name +
+ "', did you intend to nest?");
+ }
passes.emplace_back(std::move(pass));
}
@@ -240,14 +251,14 @@ void OpPassManagerImpl::splitAdaptorPasses() {
// OpPassManager
//===----------------------------------------------------------------------===//
-OpPassManager::OpPassManager(Identifier name)
- : impl(new OpPassManagerImpl(name)) {}
-OpPassManager::OpPassManager(StringRef name)
- : impl(new OpPassManagerImpl(name)) {}
+OpPassManager::OpPassManager(Identifier name, Nesting nesting)
+ : impl(new OpPassManagerImpl(name, nesting)) {}
+OpPassManager::OpPassManager(StringRef name, Nesting nesting)
+ : impl(new OpPassManagerImpl(name, nesting)) {}
OpPassManager::OpPassManager(OpPassManager &&rhs) : impl(std::move(rhs.impl)) {}
OpPassManager::OpPassManager(const OpPassManager &rhs) { *this = rhs; }
OpPassManager &OpPassManager::operator=(const OpPassManager &rhs) {
- impl.reset(new OpPassManagerImpl(rhs.impl->name));
+ impl.reset(new OpPassManagerImpl(rhs.impl->name, rhs.impl->nesting));
for (auto &pass : rhs.impl->passes)
impl->passes.emplace_back(pass->clone());
return *this;
@@ -783,8 +794,9 @@ PassManager::runWithCrashRecovery(MutableArrayRef<std::unique_ptr<Pass>> passes,
// PassManager
//===----------------------------------------------------------------------===//
-PassManager::PassManager(MLIRContext *ctx)
- : OpPassManager(Identifier::get(ModuleOp::getOperationName(), ctx)),
+PassManager::PassManager(MLIRContext *ctx, Nesting nesting)
+ : OpPassManager(Identifier::get(ModuleOp::getOperationName(), ctx),
+ nesting),
context(ctx), passTiming(false), localReproducer(false),
verifyPasses(true) {}
diff --git a/mlir/lib/Support/MlirOptMain.cpp b/mlir/lib/Support/MlirOptMain.cpp
index 3752cf4f3a13..7b9470c0d630 100644
--- a/mlir/lib/Support/MlirOptMain.cpp
+++ b/mlir/lib/Support/MlirOptMain.cpp
@@ -58,7 +58,7 @@ static LogicalResult performActions(raw_ostream &os, bool verifyDiagnostics,
return failure();
// Apply any pass manager command line options.
- PassManager pm(context);
+ PassManager pm(context, OpPassManager::Nesting::Implicit);
pm.enableVerifier(verifyPasses);
applyPassManagerCLOptions(pm);
diff --git a/mlir/test/lib/Transforms/TestConvVectorization.cpp b/mlir/test/lib/Transforms/TestConvVectorization.cpp
index e5aa49ec931d..196f3ad841db 100644
--- a/mlir/test/lib/Transforms/TestConvVectorization.cpp
+++ b/mlir/test/lib/Transforms/TestConvVectorization.cpp
@@ -100,7 +100,7 @@ void TestConvVectorization::runOnOperation() {
// Programmatic controlled lowering of linalg.copy and linalg.fill.
PassManager pm(context);
- pm.addPass(createConvertLinalgToLoopsPass());
+ pm.addNestedPass<FuncOp>(createConvertLinalgToLoopsPass());
if (failed(pm.run(module)))
llvm_unreachable("Unexpected failure in linalg to loops pass.");
diff --git a/mlir/test/lib/Transforms/TestDynamicPipeline.cpp b/mlir/test/lib/Transforms/TestDynamicPipeline.cpp
index b7d6f1227bc0..21e89a3a6165 100644
--- a/mlir/test/lib/Transforms/TestDynamicPipeline.cpp
+++ b/mlir/test/lib/Transforms/TestDynamicPipeline.cpp
@@ -25,7 +25,8 @@ class TestDynamicPipelinePass
: public PassWrapper<TestDynamicPipelinePass, OperationPass<>> {
public:
void getDependentDialects(DialectRegistry ®istry) const override {
- OpPassManager pm(ModuleOp::getOperationName());
+ OpPassManager pm(ModuleOp::getOperationName(),
+ OpPassManager::Nesting::Implicit);
parsePassPipeline(pipeline, pm, llvm::errs());
pm.getDependentDialects(registry);
}
@@ -54,7 +55,8 @@ class TestDynamicPipelinePass
}
if (!pm) {
pm = std::make_unique<OpPassManager>(
- getOperation()->getName().getIdentifier());
+ getOperation()->getName().getIdentifier(),
+ OpPassManager::Nesting::Implicit);
parsePassPipeline(pipeline, *pm, llvm::errs());
}
diff --git a/mlir/unittests/Pass/PassManagerTest.cpp b/mlir/unittests/Pass/PassManagerTest.cpp
index 99d4972ef63c..cf32fe9e424f 100644
--- a/mlir/unittests/Pass/PassManagerTest.cpp
+++ b/mlir/unittests/Pass/PassManagerTest.cpp
@@ -108,13 +108,16 @@ TEST(PassManagerTest, InvalidPass) {
// Instantiate and run our pass.
PassManager pm(&context);
- pm.addPass(std::make_unique<InvalidPass>());
+ pm.nest("invalid_op").addPass(std::make_unique<InvalidPass>());
LogicalResult result = pm.run(module.get());
EXPECT_TRUE(failed(result));
ASSERT_TRUE(diagnostic.get() != nullptr);
EXPECT_EQ(
diagnostic->str(),
"'invalid_op' op trying to schedule a pass on an unregistered operation");
+
+ // Check that adding the pass at the top-level triggers a fatal error.
+ ASSERT_DEATH(pm.addPass(std::make_unique<InvalidPass>()), "");
}
} // end namespace
More information about the Mlir-commits
mailing list