[Mlir-commits] [mlir] 0c36a15 - [mlir][Pass] Disallow mixing -pass-pipeline with other pass options
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Wed Nov 2 09:16:55 PDT 2022
Author: rkayaith
Date: 2022-11-02T12:16:50-04:00
New Revision: 0c36a1569a066e2a5fcbaa2835c7b3ea49f60458
URL: https://github.com/llvm/llvm-project/commit/0c36a1569a066e2a5fcbaa2835c7b3ea49f60458
DIFF: https://github.com/llvm/llvm-project/commit/0c36a1569a066e2a5fcbaa2835c7b3ea49f60458.diff
LOG: [mlir][Pass] Disallow mixing -pass-pipeline with other pass options
Currently `-pass-pipeline` can be specified multiple times and mixed
with the individual `-pass-name` options. Removing this feature will
allow for including the pipeline anchor as part of the option
argument (see D134900).
Reviewed By: rriddle
Differential Revision: https://reviews.llvm.org/D135745
Added:
Modified:
mlir/include/mlir/Pass/PassRegistry.h
mlir/lib/Pass/PassRegistry.cpp
mlir/test/Pass/pipeline-parsing.mlir
Removed:
################################################################################
diff --git a/mlir/include/mlir/Pass/PassRegistry.h b/mlir/include/mlir/Pass/PassRegistry.h
index 4f261e533ad15..97692262acc8e 100644
--- a/mlir/include/mlir/Pass/PassRegistry.h
+++ b/mlir/include/mlir/Pass/PassRegistry.h
@@ -231,7 +231,8 @@ struct PassPipelineCLParserImpl;
/// options for each of the passes and pipelines that have been registered with
/// the pass registry; Meaning that `-cse` will refer to the CSE pass in MLIR.
/// It also registers an argument, `pass-pipeline`, that supports parsing a
-/// textual description of a pipeline.
+/// textual description of a pipeline. This option is mutually exclusive with
+/// the individual pass options.
class PassPipelineCLParser {
public:
/// Construct a pass pipeline parser with the given command line description.
@@ -254,6 +255,8 @@ class PassPipelineCLParser {
private:
std::unique_ptr<detail::PassPipelineCLParserImpl> impl;
+
+ llvm::cl::opt<std::string> passPipeline;
};
/// This class implements a command-line parser specifically for MLIR pass
diff --git a/mlir/lib/Pass/PassRegistry.cpp b/mlir/lib/Pass/PassRegistry.cpp
index 423c97c7a6466..31b41153874a8 100644
--- a/mlir/lib/Pass/PassRegistry.cpp
+++ b/mlir/lib/Pass/PassRegistry.cpp
@@ -12,6 +12,7 @@
#include "mlir/Pass/PassManager.h"
#include "mlir/Pass/PassRegistry.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/Support/Format.h"
#include "llvm/Support/ManagedStatic.h"
#include "llvm/Support/MemoryBuffer.h"
@@ -532,6 +533,13 @@ LogicalResult TextualPipeline::initialize(StringRef text,
LogicalResult TextualPipeline::addToPipeline(
OpPassManager &pm,
function_ref<LogicalResult(const Twine &)> errorHandler) const {
+ // Temporarily disable implicit nesting while we append to the pipeline. We
+ // want the created pipeline to exactly match the parsed text pipeline, so
+ // it's preferrable to just error out if implicit nesting would be required.
+ OpPassManager::Nesting nesting = pm.getNesting();
+ pm.setNesting(OpPassManager::Nesting::Explicit);
+ auto restore = llvm::make_scope_exit([&]() { pm.setNesting(nesting); });
+
return addToPipeline(pipeline, pm, errorHandler);
}
@@ -730,10 +738,6 @@ struct PassArgData {
/// This field is set when instance specific pass options have been provided
/// on the command line.
StringRef options;
-
- /// This field is used when the parsed option corresponds to an explicit
- /// pipeline.
- TextualPipeline pipeline;
};
} // namespace
@@ -775,9 +779,8 @@ struct PassNameParser : public llvm::cl::parser<PassArgData> {
PassArgData &value);
/// If true, this parser only parses entries that correspond to a concrete
- /// pass registry entry, and does not add a `pass-pipeline` argument, does not
- /// include the options for pass entries, and does not include pass pipelines
- /// entries.
+ /// pass registry entry, and does not include pipeline entries or the options
+ /// for pass entries.
bool passNamesOnly = false;
};
} // namespace
@@ -785,12 +788,6 @@ struct PassNameParser : public llvm::cl::parser<PassArgData> {
void PassNameParser::initialize() {
llvm::cl::parser<PassArgData>::initialize();
- /// Add an entry for the textual pass pipeline option.
- if (!passNamesOnly) {
- addLiteralOption(passPipelineArg, PassArgData(),
- "A textual description of a pass pipeline to run");
- }
-
/// Add the pass entries.
for (const auto &kv : *passRegistry) {
addLiteralOption(kv.second.getPassArgument(), &kv.second,
@@ -823,11 +820,6 @@ void PassNameParser::printOptionInfo(const llvm::cl::Option &opt,
llvm::outs() << " " << opt.HelpStr << '\n';
}
- // Print the top-level pipeline argument.
- printOptionHelp(passPipelineArg,
- "A textual description of a pass pipeline to run",
- /*indent=*/4, globalWidth, /*isTopLevel=*/!opt.hasArgStr());
-
// Functor used to print the ordered entries of a registration map.
auto printOrderedEntries = [&](StringRef header, auto &map) {
llvm::SmallVector<PassRegistryEntry *, 32> orderedEntries;
@@ -865,11 +857,6 @@ size_t PassNameParser::getOptionWidth(const llvm::cl::Option &opt) const {
bool PassNameParser::parse(llvm::cl::Option &opt, StringRef argName,
StringRef arg, PassArgData &value) {
- // Handle the pipeline option explicitly.
- if (argName == passPipelineArg)
- return failed(value.pipeline.initialize(arg, llvm::errs()));
-
- // Otherwise, default to the base for handling.
if (llvm::cl::parser<PassArgData>::parse(opt, argName, arg, value))
return true;
value.options = arg;
@@ -907,12 +894,16 @@ struct PassPipelineCLParserImpl {
/// Construct a pass pipeline parser with the given command line description.
PassPipelineCLParser::PassPipelineCLParser(StringRef arg, StringRef description)
: impl(std::make_unique<detail::PassPipelineCLParserImpl>(
- arg, description, /*passNamesOnly=*/false)) {}
+ arg, description, /*passNamesOnly=*/false)),
+ passPipeline(
+ StringRef(passPipelineArg),
+ llvm::cl::desc("Textual description of the pass pipeline to run")) {}
PassPipelineCLParser::~PassPipelineCLParser() = default;
/// Returns true if this parser contains any valid options to add.
bool PassPipelineCLParser::hasAnyOccurrences() const {
- return impl->passList.getNumOccurrences() != 0;
+ return passPipeline.getNumOccurrences() != 0 ||
+ impl->passList.getNumOccurrences() != 0;
}
/// Returns true if the given pass registry entry was registered at the
@@ -925,19 +916,22 @@ bool PassPipelineCLParser::contains(const PassRegistryEntry *entry) const {
LogicalResult PassPipelineCLParser::addToPipeline(
OpPassManager &pm,
function_ref<LogicalResult(const Twine &)> errorHandler) const {
+ if (passPipeline.getNumOccurrences()) {
+ if (impl->passList.getNumOccurrences())
+ return errorHandler(
+ "'-" + passPipelineArg +
+ "' option can't be used with individual pass options");
+ std::string errMsg;
+ llvm::raw_string_ostream os(errMsg);
+ if (failed(parsePassPipeline(passPipeline, pm, os)))
+ return errorHandler(errMsg);
+ return success();
+ }
+
for (auto &passIt : impl->passList) {
- if (passIt.registryEntry) {
- if (failed(passIt.registryEntry->addToPipeline(pm, passIt.options,
- errorHandler)))
- return failure();
- } else {
- OpPassManager::Nesting nesting = pm.getNesting();
- pm.setNesting(OpPassManager::Nesting::Explicit);
- LogicalResult status = passIt.pipeline.addToPipeline(pm, errorHandler);
- pm.setNesting(nesting);
- if (failed(status))
- return failure();
- }
+ if (failed(passIt.registryEntry->addToPipeline(pm, passIt.options,
+ errorHandler)))
+ return failure();
}
return success();
}
diff --git a/mlir/test/Pass/pipeline-parsing.mlir b/mlir/test/Pass/pipeline-parsing.mlir
index 0ca4897988ee1..3e7ce7cb68020 100644
--- a/mlir/test/Pass/pipeline-parsing.mlir
+++ b/mlir/test/Pass/pipeline-parsing.mlir
@@ -13,6 +13,9 @@
// CHECK_ERROR_4: does not refer to a registered pass or pass pipeline
// CHECK_ERROR_5: Can't add pass '{{.*}}TestModulePass' restricted to 'builtin.module' on a PassManager intended to run on 'func.func', did you intend to nest?
+// RUN: not mlir-opt %s -pass-pipeline='' -cse 2>&1 | FileCheck --check-prefix=CHECK_ERROR_6 %s
+// CHECK_ERROR_6: '-pass-pipeline' option can't be used with individual pass options
+
func.func @foo() {
return
}
More information about the Mlir-commits
mailing list