diff --git a/flang/include/flang/Tools/CLOptions.inc b/flang/include/flang/Tools/CLOptions.inc --- a/flang/include/flang/Tools/CLOptions.inc +++ b/flang/include/flang/Tools/CLOptions.inc @@ -92,10 +92,14 @@ namespace fir { -static void defaultFlangInlinerOptPipeline(mlir::OpPassManager &pm) { +static mlir::OpPassManager buildDefaultFlangInlinerOptPipeline() { + mlir::OpPassManager pm; + mlir::GreedyRewriteConfig config; config.enableRegionSimplification = false; pm.addPass(mlir::createCanonicalizerPass(config)); + + return pm; } inline void addCfgConversionPass(mlir::PassManager &pm) { @@ -176,8 +180,8 @@ // The default inliner pass adds the canonicalizer pass with the default // configuration. Create the inliner pass with tco config. llvm::StringMap pipelines; - pm.addPass( - mlir::createInlinerPass(pipelines, defaultFlangInlinerOptPipeline)); + pm.addPass(mlir::createInlinerPass( + pipelines, buildDefaultFlangInlinerOptPipeline())); pm.addPass(fir::createSimplifyRegionLitePass()); pm.addPass(mlir::createCSEPass()); diff --git a/mlir/include/mlir/Pass/PassManager.h b/mlir/include/mlir/Pass/PassManager.h --- a/mlir/include/mlir/Pass/PassManager.h +++ b/mlir/include/mlir/Pass/PassManager.h @@ -75,6 +75,7 @@ OpPassManager(const OpPassManager &rhs); ~OpPassManager(); OpPassManager &operator=(const OpPassManager &rhs); + OpPassManager &operator=(OpPassManager &&rhs); /// Iterator over the passes in this pass manager. using pass_iterator = diff --git a/mlir/include/mlir/Pass/PassOptions.h b/mlir/include/mlir/Pass/PassOptions.h --- a/mlir/include/mlir/Pass/PassOptions.h +++ b/mlir/include/mlir/Pass/PassOptions.h @@ -461,25 +461,12 @@ template <> class parser : public basic_parser { public: - /// A utility struct used when parsing a pass manager that prevents the need - /// for a default constructor on OpPassManager. - struct ParsedPassManager { - ParsedPassManager(); - ParsedPassManager(ParsedPassManager &&); - ~ParsedPassManager(); - operator const mlir::OpPassManager &() const { - assert(value && "parsed value was invalid"); - return *value; - } - - std::unique_ptr value; - }; - using parser_data_type = ParsedPassManager; + using parser_data_type = mlir::OpPassManager; using OptVal = OptionValue; parser(Option &opt) : basic_parser(opt) {} - bool parse(Option &, StringRef, StringRef arg, ParsedPassManager &value); + bool parse(Option &, StringRef, StringRef arg, mlir::OpPassManager &value); /// Print an instance of the underling option value to the given stream. static void print(raw_ostream &os, const mlir::OpPassManager &value); @@ -487,7 +474,7 @@ // Overload in subclass to provide a better default value. StringRef getValueName() const override { return "pass-manager"; } - void printOptionDiff(const Option &opt, mlir::OpPassManager &pm, + void printOptionDiff(const Option &opt, const mlir::OpPassManager &pm, const OptVal &defaultValue, size_t globalWidth) const; // An out-of-line virtual method to provide a 'home' for this class. diff --git a/mlir/include/mlir/Transforms/Passes.h b/mlir/include/mlir/Transforms/Passes.h --- a/mlir/include/mlir/Transforms/Passes.h +++ b/mlir/include/mlir/Transforms/Passes.h @@ -15,6 +15,7 @@ #define MLIR_TRANSFORMS_PASSES_H #include "mlir/Pass/Pass.h" +#include "mlir/Pass/PassManager.h" #include "mlir/Transforms/LocationSnapshot.h" #include "mlir/Transforms/ViewOpGraph.h" #include "llvm/Support/Debug.h" @@ -91,10 +92,10 @@ /// Creates an instance of the inliner pass, and use the provided pass managers /// when optimizing callable operations with names matching the key type. /// Callable operations with a name not within the provided map will use the -/// provided default pipeline builder. +/// provided default pipeline, which must be anchored on `any`. std::unique_ptr createInlinerPass(llvm::StringMap opPipelines, - std::function defaultPipelineBuilder); + OpPassManager defaultPipeline); /// Creates a pass which performs sparse conditional constant propagation over /// nested operations. diff --git a/mlir/include/mlir/Transforms/Passes.td b/mlir/include/mlir/Transforms/Passes.td --- a/mlir/include/mlir/Transforms/Passes.td +++ b/mlir/include/mlir/Transforms/Passes.td @@ -81,8 +81,8 @@ let summary = "Inline function calls"; let constructor = "mlir::createInlinerPass()"; let options = [ - Option<"defaultPipelineStr", "default-pipeline", "std::string", - /*default=*/"", "The default optimizer pipeline used for callables">, + Option<"defaultPipeline", "default-pipeline", "OpPassManager", + /*default=*/"", "The default optimizer pipeline used for callables (must be anchored on 'any')">, ListOption<"opPipelineList", "op-pipelines", "OpPassManager", "Callable operation specific optimizer pipelines (in the form " "of `dialect.op(pipeline)`)">, diff --git a/mlir/lib/Pass/Pass.cpp b/mlir/lib/Pass/Pass.cpp --- a/mlir/lib/Pass/Pass.cpp +++ b/mlir/lib/Pass/Pass.cpp @@ -300,6 +300,10 @@ impl = std::make_unique(*rhs.impl); return *this; } +OpPassManager &OpPassManager::operator=(OpPassManager &&rhs) { + impl = std::move(rhs.impl); + return *this; +} OpPassManager::~OpPassManager() = default; 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 @@ -397,25 +397,30 @@ } // namespace llvm bool llvm::cl::parser::parse(Option &, StringRef, StringRef arg, - ParsedPassManager &value) { + OpPassManager &value) { + if (arg.empty()) + return false; + FailureOr pipeline = parsePassPipeline(arg); if (failed(pipeline)) return true; - value.value = std::make_unique(std::move(*pipeline)); + value = std::move(*pipeline); return false; } void llvm::cl::parser::print(raw_ostream &os, const OpPassManager &value) { + os << value.getOpAnchorName() << "("; value.printAsTextualPipeline(os); + os << ")"; } void llvm::cl::parser::printOptionDiff( - const Option &opt, OpPassManager &pm, const OptVal &defaultValue, + const Option &opt, const OpPassManager &pm, const OptVal &defaultValue, size_t globalWidth) const { printOptionName(opt, globalWidth); outs() << "= "; - pm.printAsTextualPipeline(outs()); + print(outs(), pm); if (defaultValue.hasValue()) { outs().indent(2) << " (default: "; @@ -427,13 +432,6 @@ void llvm::cl::parser::anchor() {} -llvm::cl::parser::ParsedPassManager::ParsedPassManager() = - default; -llvm::cl::parser::ParsedPassManager::ParsedPassManager( - ParsedPassManager &&) = default; -llvm::cl::parser::ParsedPassManager::~ParsedPassManager() = - default; - //===----------------------------------------------------------------------===// // TextualPassPipeline Parser //===----------------------------------------------------------------------===// diff --git a/mlir/lib/Transforms/Inliner.cpp b/mlir/lib/Transforms/Inliner.cpp --- a/mlir/lib/Transforms/Inliner.cpp +++ b/mlir/lib/Transforms/Inliner.cpp @@ -35,8 +35,10 @@ using namespace mlir; /// This function implements the default inliner optimization pipeline. -static void defaultInlinerOptPipeline(OpPassManager &pm) { +static OpPassManager buildDefaultInlinerOptPipeline() { + OpPassManager pm; pm.addPass(createCanonicalizerPass()); + return pm; } //===----------------------------------------------------------------------===// @@ -591,8 +593,8 @@ public: InlinerPass(); InlinerPass(const InlinerPass &) = default; - InlinerPass(std::function defaultPipeline); - InlinerPass(std::function defaultPipeline, + InlinerPass(OpPassManager &&defaultPM); + InlinerPass(OpPassManager &&defaultPM, llvm::StringMap opPipelines); void runOnOperation() override; @@ -627,9 +629,6 @@ /// class variant. LogicalResult initializeOptions(StringRef options) override; - /// An optional function that constructs a default optimization pipeline for - /// a given operation. - std::function defaultPipeline; /// A map of operation names to pass pipelines to use when optimizing /// callable operations of these types. This provides a specialized pipeline /// instead of the default. The vector size is the number of threads used @@ -638,23 +637,15 @@ }; } // namespace -InlinerPass::InlinerPass() : InlinerPass(defaultInlinerOptPipeline) {} -InlinerPass::InlinerPass(std::function defaultPipeline) - : defaultPipeline(std::move(defaultPipeline)) { +InlinerPass::InlinerPass() : InlinerPass(OpPassManager()) {} +InlinerPass::InlinerPass(OpPassManager &&defaultPM) { + defaultPipeline = std::move(defaultPM); opPipelines.push_back({}); - - // Initialize the pass options with the provided arguments. - if (defaultPipeline) { - OpPassManager fakePM("__mlir_fake_pm_op"); - defaultPipeline(fakePM); - llvm::raw_string_ostream strStream(defaultPipelineStr); - fakePM.printAsTextualPipeline(strStream); - } } -InlinerPass::InlinerPass(std::function defaultPipeline, +InlinerPass::InlinerPass(OpPassManager &&defaultPM, llvm::StringMap opPipelines) - : InlinerPass(std::move(defaultPipeline)) { + : InlinerPass(std::move(defaultPM)) { if (opPipelines.empty()) return; @@ -791,13 +782,13 @@ Operation *callable = node->getCallableRegion()->getParentOp(); StringRef opName = callable->getName().getStringRef(); auto pipelineIt = pipelines.find(opName); + + // If a pipeline didn't exist, use the default if possible. if (pipelineIt == pipelines.end()) { - // If a pipeline didn't exist, use the default if possible. - if (!defaultPipeline) + if (defaultPipeline.empty()) return success(); - OpPassManager defaultPM(opName); - defaultPipeline(defaultPM); + OpPassManager defaultPM(defaultPipeline); pipelineIt = pipelines.try_emplace(opName, std::move(defaultPM)).first; } return runPipeline(pipelineIt->second, callable); @@ -807,15 +798,15 @@ if (failed(Pass::initializeOptions(options))) return failure(); - // Initialize the default pipeline builder to use the option string. - // TODO: Use a generic pass manager for default pipelines, and remove this. - if (!defaultPipelineStr.empty()) { - std::string defaultPipelineCopy = defaultPipelineStr; - defaultPipeline = [=](OpPassManager &pm) { - (void)parsePassPipeline(defaultPipelineCopy, pm); - }; - } else if (defaultPipelineStr.getNumOccurrences()) { - defaultPipeline = nullptr; + // Initialize the default pipeline builder. + if (!defaultPipeline.getNumOccurrences()) { + defaultPipeline = buildDefaultInlinerOptPipeline(); + } else if (defaultPipeline.getValue().getOpAnchorName() != + OpPassManager::getAnyOpAnchorName()) { + llvm::errs() << "error: Inliner expected default pipeline to be anchored " + "on 'any', got '" + << defaultPipeline.getValue().getOpAnchorName() << "'"; + return failure(); } // Initialize the op specific pass pipelines. @@ -833,12 +824,12 @@ } std::unique_ptr mlir::createInlinerPass(llvm::StringMap opPipelines) { - return std::make_unique(defaultInlinerOptPipeline, + return std::make_unique(buildDefaultInlinerOptPipeline(), std::move(opPipelines)); } -std::unique_ptr mlir::createInlinerPass( - llvm::StringMap opPipelines, - std::function defaultPipelineBuilder) { - return std::make_unique(std::move(defaultPipelineBuilder), +std::unique_ptr +mlir::createInlinerPass(llvm::StringMap opPipelines, + OpPassManager defaultPipeline) { + return std::make_unique(std::move(defaultPipeline), std::move(opPipelines)); } diff --git a/mlir/test/Dialect/Affine/inlining.mlir b/mlir/test/Dialect/Affine/inlining.mlir --- a/mlir/test/Dialect/Affine/inlining.mlir +++ b/mlir/test/Dialect/Affine/inlining.mlir @@ -1,4 +1,4 @@ -// RUN: mlir-opt -allow-unregistered-dialect %s -inline="default-pipeline=''" | FileCheck %s +// RUN: mlir-opt -allow-unregistered-dialect %s -inline="default-pipeline=" | FileCheck %s // Basic test that functions within affine operations are inlined. func.func @func_with_affine_ops(%N: index) { diff --git a/mlir/test/Transforms/inlining.mlir b/mlir/test/Transforms/inlining.mlir --- a/mlir/test/Transforms/inlining.mlir +++ b/mlir/test/Transforms/inlining.mlir @@ -3,6 +3,9 @@ // RUN: mlir-opt %s -inline='default-pipeline=''' -mlir-print-debuginfo -mlir-print-local-scope | FileCheck %s --check-prefix INLINE-LOC // RUN: mlir-opt %s -inline | FileCheck %s --check-prefix INLINE_SIMPLIFY // RUN: mlir-opt %s -inline='op-pipelines=func.func(canonicalize,cse)' | FileCheck %s --check-prefix INLINE_SIMPLIFY +// RUN: not mlir-opt %s -inline='default-pipeline=func.func(canonicalize,cse)' 2>&1 | FileCheck %s --check-prefix INLINE_INVALID_DEFAULT + +// INLINE_INVALID_DEFAULT: error: Inliner expected default pipeline to be anchored on 'any', got 'func.func' // Inline a function that takes an argument. func.func @func_with_arg(%c : i32) -> i32 {