diff --git a/mlir/include/mlir/Pass/PassRegistry.h b/mlir/include/mlir/Pass/PassRegistry.h --- a/mlir/include/mlir/Pass/PassRegistry.h +++ b/mlir/include/mlir/Pass/PassRegistry.h @@ -231,7 +231,8 @@ /// 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 @@ private: std::unique_ptr impl; + + llvm::cl::opt 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 --- 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::addToPipeline( OpPassManager &pm, function_ref 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); } @@ -729,10 +737,6 @@ /// 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 @@ -774,9 +778,8 @@ 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 does not include the options for pass + /// entries, and does not include pass pipelines entries. bool passNamesOnly = false; }; } // namespace @@ -784,12 +787,6 @@ void PassNameParser::initialize() { llvm::cl::parser::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, @@ -822,11 +819,6 @@ 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 orderedEntries; @@ -864,11 +856,6 @@ 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::parse(opt, argName, arg, value)) return true; value.options = arg; @@ -906,12 +893,16 @@ /// Construct a pass pipeline parser with the given command line description. PassPipelineCLParser::PassPipelineCLParser(StringRef arg, StringRef description) : impl(std::make_unique( - 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 @@ -924,19 +915,18 @@ LogicalResult PassPipelineCLParser::addToPipeline( OpPassManager &pm, function_ref errorHandler) const { + if (passPipeline.getNumOccurrences()) { + if (impl->passList.getNumOccurrences()) + return errorHandler("'-" + passPipelineArg + + "' option can't be used with individual pass options"); + // FIXME: Report errors to `errorHandler` instead of stderr + return parsePassPipeline(passPipeline, pm, llvm::errs()); + } + 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 --- 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 }