[Mlir-commits] [mlir] 7f00ba0 - Restore mlir-opt `--run-reproducer` option to opt-in running a reproducer

[Mehdi Amini]

Author: Mehdi Amini
Date: 2023-05-04T11:08:03-07:00
New Revision: 7f00ba08aade787da6ef891156315698959e7af8

URL: https://github.com/llvm/llvm-project/commit/7f00ba08aade787da6ef891156315698959e7af8
DIFF: https://github.com/llvm/llvm-project/commit/7f00ba08aade787da6ef891156315698959e7af8.diff

LOG: Restore mlir-opt `--run-reproducer` option to opt-in running a reproducer

When tooling out there produces a reproducer that is archived, the first thing
a user is likely to expect is to process this as they do with any MLIR file.
However https://reviews.llvm.org/D126447 changed the behavior of mlir-opt to
eliminate the `--run-reproducer` option and instead automatically run it when
present in the input file. This creates a discrepancy in how mlir-opt behaves
when fed with an input file, and is surprising for users.
The explicit passing of `--run-reproducer` to express user-intent seems more
in line with what is expected from `mlir-opt`.

Reviewed By: rriddle, jpienaar

Differential Revision: https://reviews.llvm.org/D149820

Added: 
    

Modified: 
    mlir/docs/PassManagement.md
    mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
    mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
    mlir/test/Pass/run-reproducer.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/docs/PassManagement.md b/mlir/docs/PassManagement.md
index 661592b13782c..9a7cfd1f9bebc 100644
--- a/mlir/docs/PassManagement.md
+++ b/mlir/docs/PassManagement.md
@@ -1333,9 +1333,9 @@ module {
 #-}
 ```
 
-The configuration dumped can be passed to `mlir-opt`. This will result in
-parsing the configuration of the reproducer and adjusting the necessary opt
-state, e.g. configuring the pass manager, context, etc.
+The configuration dumped can be passed to `mlir-opt` by specifying
+`-run-reproducer` flag. This will result in parsing the configuration of the reproducer
+and adjusting the necessary opt state, e.g. configuring the pass manager, context, etc.
 
 Beyond specifying a filename, one can also register a `ReproducerStreamFactory`
 function that would be invoked in the case of a crash and the reproducer written

diff  --git a/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h b/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
index 0cba2147a6e08..8f1969cd3ad36 100644
--- a/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
+++ b/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
@@ -116,6 +116,16 @@ class MlirOptMainConfig {
     return success();
   }
 
+  /// Enable running the reproducer information stored in resources (if
+  /// present).
+  MlirOptMainConfig &runReproducer(bool enableReproducer) {
+    runReproducerFlag = enableReproducer;
+    return *this;
+  };
+
+  /// Return true if the reproducer should be run.
+  bool shouldRunReproducer() const { return runReproducerFlag; }
+
   /// Show the registered dialects before trying to load the input file.
   MlirOptMainConfig &showDialects(bool show) {
     showDialectsFlag = show;
@@ -183,6 +193,9 @@ class MlirOptMainConfig {
   /// The callback to populate the pass manager.
   std::function<LogicalResult(PassManager &)> passPipelineCallback;
 
+  /// Enable running the reproducer.
+  bool runReproducerFlag = false;
+
   /// Show the registered dialects before trying to load the input file.
   bool showDialectsFlag = false;
 

diff  --git a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
index 18060edb2ed7a..2aad687e41957 100644
--- a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
+++ b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
@@ -113,6 +113,10 @@ struct MlirOptMainConfigCLOptions : public MlirOptMainConfig {
                  "parsing"),
         cl::location(useExplicitModuleFlag), cl::init(false));
 
+    static cl::opt<bool, /*ExternalStorage=*/true> runReproducer(
+        "run-reproducer", cl::desc("Run the pipeline stored in the reproducer"),
+        cl::location(runReproducerFlag), cl::init(false));
+
     static cl::opt<bool, /*ExternalStorage=*/true> showDialects(
         "show-dialects",
         cl::desc("Print the list of registered dialects and exit"),
@@ -236,7 +240,8 @@ performActions(raw_ostream &os,
   FallbackAsmResourceMap fallbackResourceMap;
   ParserConfig parseConfig(context, /*verifyAfterParse=*/true,
                            &fallbackResourceMap);
-  reproOptions.attachResourceParser(parseConfig);
+  if (config.shouldRunReproducer())
+    reproOptions.attachResourceParser(parseConfig);
 
   // Parse the input file and reset the context threading state.
   TimingScope parserTiming = timing.nest("Parser");
@@ -253,7 +258,9 @@ performActions(raw_ostream &os,
   if (failed(applyPassManagerCLOptions(pm)))
     return failure();
   pm.enableTiming(timing);
-  if (failed(reproOptions.apply(pm)) || failed(config.setupPassPipeline(pm)))
+  if (config.shouldRunReproducer() && failed(reproOptions.apply(pm)))
+    return failure();
+  if (failed(config.setupPassPipeline(pm)))
     return failure();
 
   // Run the pipeline.

diff  --git a/mlir/test/Pass/run-reproducer.mlir b/mlir/test/Pass/run-reproducer.mlir
index 903fd69458efd..57a58dbaa5b96 100644
--- a/mlir/test/Pass/run-reproducer.mlir
+++ b/mlir/test/Pass/run-reproducer.mlir
@@ -1,5 +1,5 @@
-// RUN: mlir-opt %s -dump-pass-pipeline 2>&1 | FileCheck %s
-// RUN: mlir-opt %s -mlir-print-ir-before=cse 2>&1 | FileCheck -check-prefix=BEFORE %s
+// RUN: mlir-opt %s --run-reproducer -dump-pass-pipeline 2>&1 | FileCheck %s
+// RUN: mlir-opt %s --run-reproducer -mlir-print-ir-before=cse 2>&1 | FileCheck -check-prefix=BEFORE %s
 
 func.func @foo() {
   %0 = arith.constant 0 : i32