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 @@ -21,7 +21,9 @@ namespace mlir { class OpPassManager; +class AsmParserConfig; class Pass; +class PassManager; namespace detail { class PassOptions; @@ -272,6 +274,18 @@ std::unique_ptr impl; }; +//===----------------------------------------------------------------------===// +// Pass Reproducer +//===----------------------------------------------------------------------===// + +/// Attach an assembly configuration parser that handles MLIR reproducer +/// configurations. Any found reproducer information will be attached to the +/// given pass manager, e.g. the reproducer pipeline, verification flags, etc. +// FIXME: Remove the `enableThreading` flag when possible. Some tools, e.g. +// mlir-opt, force disable threading during parsing. +void attachPassReproducerAsmConfig(AsmParserConfig &config, PassManager &pm, + bool &enableThreading); + } // namespace mlir #endif // MLIR_PASS_PASSREGISTRY_H_ diff --git a/mlir/lib/Pass/PassCrashRecovery.cpp b/mlir/lib/Pass/PassCrashRecovery.cpp --- a/mlir/lib/Pass/PassCrashRecovery.cpp +++ b/mlir/lib/Pass/PassCrashRecovery.cpp @@ -11,6 +11,7 @@ #include "mlir/IR/Dialect.h" #include "mlir/IR/SymbolTable.h" #include "mlir/IR/Verifier.h" +#include "mlir/Parser/Parser.h" #include "mlir/Pass/Pass.h" #include "mlir/Support/FileUtilities.h" #include "llvm/ADT/STLExtras.h" @@ -117,17 +118,16 @@ } descOS << "reproducer generated at `" << stream->description() << "`"; - // Output the current pass manager configuration to the crash stream. - auto &os = stream->os(); - os << "// configuration: -pass-pipeline='" << pipeline << "'"; - if (disableThreads) - os << " -mlir-disable-threading"; - if (verifyPasses) - os << " -verify-each"; - os << '\n'; + AsmState state(preCrashOperation); + state.attachConfigPrinter( + "mlir_reproducer", [&](Operation *op, AsmConfigBuilder &provider) { + provider.buildString("pipeline", pipeline); + provider.buildBool("disable_threading", disableThreads); + provider.buildBool("verify_each", verifyPasses); + }); // Output the .mlir module. - preCrashOperation->print(os); + preCrashOperation->print(stream->os(), state); } void RecoveryReproducerContext::disable() { @@ -438,3 +438,38 @@ addInstrumentation( std::make_unique(*crashReproGenerator)); } + +//===----------------------------------------------------------------------===// +// Asm Configuration +//===----------------------------------------------------------------------===// + +void mlir::attachPassReproducerAsmConfig(AsmParserConfig &config, + PassManager &pm, + bool &enableThreading) { + auto parseFn = [&](AsmConfigEntry &entry) -> LogicalResult { + if (entry.getKey() == "pipeline") { + FailureOr pipeline = entry.parseAsString(); + if (failed(pipeline)) + return failure(); + return parsePassPipeline(*pipeline, pm); + } + if (entry.getKey() == "disable_threading") { + FailureOr value = entry.parseAsBool(); + + // FIXME: We should just update the context directly, but some places + // force disable threading during parsing. + if (succeeded(value)) + enableThreading = !(*value); + return value; + } + if (entry.getKey() == "verify_each") { + FailureOr value = entry.parseAsBool(); + if (succeeded(value)) + pm.enableVerifier(*value); + return value; + } + return entry.emitError() << "unknown 'mlir_reproducer' configuration key '" + << entry.getKey() << "'"; + }; + config.attachConfigParser("mlir_reproducer", parseFn); +} diff --git a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp --- a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp +++ b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp @@ -57,20 +57,24 @@ bool wasThreadingEnabled = context->isMultithreadingEnabled(); context->disableMultithreading(); + // Prepare the pass manager and apply any command line options. + PassManager pm(context, OpPassManager::Nesting::Implicit); + pm.enableVerifier(verifyPasses); + applyPassManagerCLOptions(pm); + pm.enableTiming(timing); + + // Prepare the parser config, and attach any useful/necessary configurations. + AsmParserConfig config(context); + attachPassReproducerAsmConfig(config, pm, wasThreadingEnabled); + // Parse the input file and reset the context threading state. TimingScope parserTiming = timing.nest("Parser"); - OwningOpRef module(parseSourceFile(sourceMgr, context)); + OwningOpRef module(parseSourceFile(sourceMgr, config)); context->enableMultithreading(wasThreadingEnabled); if (!module) return failure(); parserTiming.stop(); - // Apply any pass manager command line options. - PassManager pm(context, OpPassManager::Nesting::Implicit); - pm.enableVerifier(verifyPasses); - applyPassManagerCLOptions(pm); - pm.enableTiming(timing); - // Callback to build the pipeline. if (failed(passManagerSetupFn(pm))) return failure(); @@ -226,11 +230,6 @@ "show-dialects", cl::desc("Print the list of registered dialects"), cl::init(false)); - static cl::opt runRepro( - "run-reproducer", - cl::desc("Append the command line options of the reproducer"), - cl::init(false)); - InitLLVM y(argc, argv); // Register any command line options. @@ -267,23 +266,6 @@ return failure(); } - // Parse reproducer options. - BumpPtrAllocator a; - StringSaver saver(a); - if (runRepro) { - auto pair = file->getBuffer().split('\n'); - if (!pair.first.consume_front("// configuration:")) { - llvm::errs() << "Failed to find repro configuration, expect file to " - "begin with '// configuration:'\n"; - return failure(); - } - // Tokenize & parse the first line. - SmallVector newArgv; - newArgv.push_back(argv[0]); - llvm::cl::TokenizeGNUCommandLine(pair.first, saver, newArgv); - cl::ParseCommandLineOptions(newArgv.size(), &newArgv[0], helpHeader); - } - auto output = openOutputFile(outputFilename, &errorMessage); if (!output) { llvm::errs() << errorMessage << "\n"; diff --git a/mlir/test/Pass/crash-recovery-dynamic-failure.mlir b/mlir/test/Pass/crash-recovery-dynamic-failure.mlir --- a/mlir/test/Pass/crash-recovery-dynamic-failure.mlir +++ b/mlir/test/Pass/crash-recovery-dynamic-failure.mlir @@ -11,7 +11,8 @@ module @foo {} } -// REPRO_LOCAL_DYNAMIC_FAILURE: configuration: -pass-pipeline='builtin.module(test-pass-failure)' // REPRO_LOCAL_DYNAMIC_FAILURE: module @inner_mod1 // REPRO_LOCAL_DYNAMIC_FAILURE: module @foo { + +// REPRO_LOCAL_DYNAMIC_FAILURE: pipeline: "builtin.module(test-pass-failure)" diff --git a/mlir/test/Pass/crash-recovery.mlir b/mlir/test/Pass/crash-recovery.mlir --- a/mlir/test/Pass/crash-recovery.mlir +++ b/mlir/test/Pass/crash-recovery.mlir @@ -20,17 +20,14 @@ module @foo {} } -// REPRO: configuration: -pass-pipeline='builtin.module(test-module-pass,test-pass-crash)' - // REPRO: module @inner_mod1 // REPRO: module @foo { - -// REPRO_LOCAL: configuration: -pass-pipeline='builtin.module(test-pass-crash)' +// REPRO: pipeline: "builtin.module(test-module-pass,test-pass-crash)" // REPRO_LOCAL: module @inner_mod1 // REPRO_LOCAL: module @foo { - -// REPRO_LOCAL_DYNAMIC: configuration: -pass-pipeline='builtin.module(test-pass-crash)' +// REPRO_LOCAL: pipeline: "builtin.module(test-pass-crash)" // REPRO_LOCAL_DYNAMIC: module @inner_mod1 // REPRO_LOCAL_DYNAMIC: module @foo { +// REPRO_LOCAL_DYNAMIC: pipeline: "builtin.module(test-pass-crash)" diff --git a/mlir/test/Pass/run-reproducer.mlir b/mlir/test/Pass/run-reproducer.mlir --- a/mlir/test/Pass/run-reproducer.mlir +++ b/mlir/test/Pass/run-reproducer.mlir @@ -1,9 +1,4 @@ -// configuration: -mlir-disable-threading=true -pass-pipeline='func.func(cse,canonicalize)' -mlir-print-ir-before=cse - -// Test of the reproducer run option. The first line has to be the -// configuration (matching what is produced by reproducer). - -// RUN: mlir-opt %s -run-reproducer 2>&1 | FileCheck -check-prefix=BEFORE %s +// RUN: mlir-opt %s -mlir-print-ir-before=cse 2>&1 | FileCheck -check-prefix=BEFORE %s func.func @foo() { %0 = arith.constant 0 : i32 @@ -14,6 +9,15 @@ return } +{-# + config: { + : { + pipeline: "func.func(cse,canonicalize)", + disable_threading: true + } + } +#-} + // BEFORE: // -----// IR Dump Before{{.*}}CSE //----- // // BEFORE-NEXT: func @foo() // BEFORE: // -----// IR Dump Before{{.*}}CSE //----- // diff --git a/mlir/test/lib/Dialect/SPIRV/TestModuleCombiner.cpp b/mlir/test/lib/Dialect/SPIRV/TestModuleCombiner.cpp --- a/mlir/test/lib/Dialect/SPIRV/TestModuleCombiner.cpp +++ b/mlir/test/lib/Dialect/SPIRV/TestModuleCombiner.cpp @@ -29,9 +29,6 @@ TestModuleCombinerPass() = default; TestModuleCombinerPass(const TestModuleCombinerPass &) {} void runOnOperation() override; - -private: - OwningOpRef combinedModule; }; } // namespace @@ -46,10 +43,12 @@ << " -> " << newSymbol << "\n"; }; - combinedModule = spirv::combine(modules, combinedModuleBuilder, listener); + OwningOpRef combinedModule = + spirv::combine(modules, combinedModuleBuilder, listener); for (spirv::ModuleOp module : modules) module.erase(); + combinedModule.release(); } namespace mlir {