[MLIR] Move the mlir-generate-reproducer option to be a PassManager option instead of mlir-opt
#159004
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-mlir Author: Mehdi Amini (joker-eph) ChangesThis makes it available to compilers in general, not limited to mlir-opt-like tools. Full diff: https://github.com/llvm/llvm-project/pull/159004.diff 5 Files Affected:
diff --git a/mlir/include/mlir/Pass/PassManager.h b/mlir/include/mlir/Pass/PassManager.h
index 6e59b0f32ac6f..6fb47dbb3492e 100644
--- a/mlir/include/mlir/Pass/PassManager.h
+++ b/mlir/include/mlir/Pass/PassManager.h
@@ -282,6 +282,11 @@ class PassManager : public OpPassManager {
/// Add the provided instrumentation to the pass manager.
void addInstrumentation(std::unique_ptr<PassInstrumentation> pi);
+ /// Enable or disable the printing of pass manager reproducer.
+ void enableGeneratePassManagerReproducer(std::string filename) {
+ generatePassManagerReproducer = std::move(filename);
+ }
+
//===--------------------------------------------------------------------===//
// IR Printing
@@ -492,6 +497,9 @@ class PassManager : public OpPassManager {
llvm::hash_code pipelineInitializationKey =
DenseMapInfo<llvm::hash_code>::getTombstoneKey();
+ /// A flag that indicates if the pass manager reproducer should be generated.
+ std::string generatePassManagerReproducer;
+
/// Flag that specifies if pass timing is enabled.
bool passTiming : 1;
diff --git a/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h b/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
index 0fbe15fa2e0db..9690ac6090fe7 100644
--- a/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
+++ b/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
@@ -237,10 +237,7 @@ class MlirOptMainConfig {
return hasFilters;
}
- /// Reproducer file generation (no crash required).
- StringRef getReproducerFilename() const { return generateReproducerFileFlag; }
-
- /// Set the reproducer output filename
+ /// Set the remarks output filename
RemarkFormat getRemarkFormat() const { return remarkFormatFlag; }
/// Set the remark format to use.
std::string getRemarksAllFilter() const { return remarksAllFilterFlag; }
@@ -340,9 +337,6 @@ class MlirOptMainConfig {
/// Verify that the input IR round-trips perfectly.
bool verifyRoundtripFlag = false;
-
- /// The reproducer output filename (no crash required).
- std::string generateReproducerFileFlag = "";
};
/// This defines the function type used to setup the pass manager. This can be
diff --git a/mlir/lib/Pass/Pass.cpp b/mlir/lib/Pass/Pass.cpp
index 521c7c6be17b6..d94bf025dfbcc 100644
--- a/mlir/lib/Pass/Pass.cpp
+++ b/mlir/lib/Pass/Pass.cpp
@@ -1033,6 +1033,14 @@ LogicalResult PassManager::run(Operation *op) {
<< size() << " passes, verifyPasses=" << verifyPasses << " pipeline: ";
printAsTextualPipeline(os, /*pretty=*/false);
});
+ // Generate reproducers if requested
+ if (!generatePassManagerReproducer.empty()) {
+ StringRef anchorName = getAnyOpAnchorName();
+ const auto &passes = getPasses();
+ makeReproducer(anchorName, passes, op, generatePassManagerReproducer,
+ /*disableThreads=*/!getContext()->isMultithreadingEnabled(),
+ verifyPasses);
+ }
MLIRContext *context = getContext();
std::optional<OperationName> anchorOp = getOpName(*context);
diff --git a/mlir/lib/Pass/PassManagerOptions.cpp b/mlir/lib/Pass/PassManagerOptions.cpp
index 305bf72bb4799..228a7716a114d 100644
--- a/mlir/lib/Pass/PassManagerOptions.cpp
+++ b/mlir/lib/Pass/PassManagerOptions.cpp
@@ -63,6 +63,11 @@ struct PassManagerOptions {
llvm::cl::desc("When printing the IR before/after a pass, print file "
"tree rooted at this directory. Use in conjunction with "
"mlir-print-ir-* flags")};
+ llvm::cl::opt<std::string> generateReproducerFile{
+ "mlir-generate-reproducer",
+ llvm::cl::desc("Generate an mlir reproducer at the provided filename"
+ " (no crash required)"),
+ llvm::cl::init(""), llvm::cl::value_desc("filename")};
/// Add an IR printing instrumentation if enabled by any 'print-ir' flags.
void addPrinterInstrumentation(PassManager &pm);
@@ -172,6 +177,9 @@ LogicalResult mlir::applyPassManagerCLOptions(PassManager &pm) {
// Add the IR printing instrumentation.
options->addPrinterInstrumentation(pm);
+
+ if (options->generateReproducerFile.getNumOccurrences())
+ pm.enableGeneratePassManagerReproducer(options->generateReproducerFile);
return success();
}
diff --git a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
index 30fd384f3977c..56932045a229d 100644
--- a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
+++ b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
@@ -198,15 +198,6 @@ struct MlirOptMainConfigCLOptions : public MlirOptMainConfig {
static cl::list<std::string> passPlugins(
"load-pass-plugin", cl::desc("Load passes from plugin library"));
- static cl::opt<std::string, /*ExternalStorage=*/true>
- generateReproducerFile(
- "mlir-generate-reproducer",
- llvm::cl::desc(
- "Generate an mlir reproducer at the provided filename"
- " (no crash required)"),
- cl::location(generateReproducerFileFlag), cl::init(""),
- cl::value_desc("filename"));
-
static cl::OptionCategory remarkCategory(
"Remark Options",
"Filter remarks by regular expression (llvm::Regex syntax).");
@@ -568,14 +559,6 @@ performActions(raw_ostream &os,
if (failed(pm.run(*op)))
return failure();
- // Generate reproducers if requested
- if (!config.getReproducerFilename().empty()) {
- StringRef anchorName = pm.getAnyOpAnchorName();
- const auto &passes = pm.getPasses();
- makeReproducer(anchorName, passes, op.get(),
- config.getReproducerFilename());
- }
-
// Print the output.
TimingScope outputTiming = timing.nest("Output");
if (config.shouldEmitBytecode()) {
|
|
@llvm/pr-subscribers-mlir-core Author: Mehdi Amini (joker-eph) ChangesThis makes it available to compilers in general, not limited to mlir-opt-like tools. Full diff: https://github.com/llvm/llvm-project/pull/159004.diff 5 Files Affected:
diff --git a/mlir/include/mlir/Pass/PassManager.h b/mlir/include/mlir/Pass/PassManager.h
index 6e59b0f32ac6f..6fb47dbb3492e 100644
--- a/mlir/include/mlir/Pass/PassManager.h
+++ b/mlir/include/mlir/Pass/PassManager.h
@@ -282,6 +282,11 @@ class PassManager : public OpPassManager {
/// Add the provided instrumentation to the pass manager.
void addInstrumentation(std::unique_ptr<PassInstrumentation> pi);
+ /// Enable or disable the printing of pass manager reproducer.
+ void enableGeneratePassManagerReproducer(std::string filename) {
+ generatePassManagerReproducer = std::move(filename);
+ }
+
//===--------------------------------------------------------------------===//
// IR Printing
@@ -492,6 +497,9 @@ class PassManager : public OpPassManager {
llvm::hash_code pipelineInitializationKey =
DenseMapInfo<llvm::hash_code>::getTombstoneKey();
+ /// A flag that indicates if the pass manager reproducer should be generated.
+ std::string generatePassManagerReproducer;
+
/// Flag that specifies if pass timing is enabled.
bool passTiming : 1;
diff --git a/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h b/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
index 0fbe15fa2e0db..9690ac6090fe7 100644
--- a/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
+++ b/mlir/include/mlir/Tools/mlir-opt/MlirOptMain.h
@@ -237,10 +237,7 @@ class MlirOptMainConfig {
return hasFilters;
}
- /// Reproducer file generation (no crash required).
- StringRef getReproducerFilename() const { return generateReproducerFileFlag; }
-
- /// Set the reproducer output filename
+ /// Set the remarks output filename
RemarkFormat getRemarkFormat() const { return remarkFormatFlag; }
/// Set the remark format to use.
std::string getRemarksAllFilter() const { return remarksAllFilterFlag; }
@@ -340,9 +337,6 @@ class MlirOptMainConfig {
/// Verify that the input IR round-trips perfectly.
bool verifyRoundtripFlag = false;
-
- /// The reproducer output filename (no crash required).
- std::string generateReproducerFileFlag = "";
};
/// This defines the function type used to setup the pass manager. This can be
diff --git a/mlir/lib/Pass/Pass.cpp b/mlir/lib/Pass/Pass.cpp
index 521c7c6be17b6..d94bf025dfbcc 100644
--- a/mlir/lib/Pass/Pass.cpp
+++ b/mlir/lib/Pass/Pass.cpp
@@ -1033,6 +1033,14 @@ LogicalResult PassManager::run(Operation *op) {
<< size() << " passes, verifyPasses=" << verifyPasses << " pipeline: ";
printAsTextualPipeline(os, /*pretty=*/false);
});
+ // Generate reproducers if requested
+ if (!generatePassManagerReproducer.empty()) {
+ StringRef anchorName = getAnyOpAnchorName();
+ const auto &passes = getPasses();
+ makeReproducer(anchorName, passes, op, generatePassManagerReproducer,
+ /*disableThreads=*/!getContext()->isMultithreadingEnabled(),
+ verifyPasses);
+ }
MLIRContext *context = getContext();
std::optional<OperationName> anchorOp = getOpName(*context);
diff --git a/mlir/lib/Pass/PassManagerOptions.cpp b/mlir/lib/Pass/PassManagerOptions.cpp
index 305bf72bb4799..228a7716a114d 100644
--- a/mlir/lib/Pass/PassManagerOptions.cpp
+++ b/mlir/lib/Pass/PassManagerOptions.cpp
@@ -63,6 +63,11 @@ struct PassManagerOptions {
llvm::cl::desc("When printing the IR before/after a pass, print file "
"tree rooted at this directory. Use in conjunction with "
"mlir-print-ir-* flags")};
+ llvm::cl::opt<std::string> generateReproducerFile{
+ "mlir-generate-reproducer",
+ llvm::cl::desc("Generate an mlir reproducer at the provided filename"
+ " (no crash required)"),
+ llvm::cl::init(""), llvm::cl::value_desc("filename")};
/// Add an IR printing instrumentation if enabled by any 'print-ir' flags.
void addPrinterInstrumentation(PassManager &pm);
@@ -172,6 +177,9 @@ LogicalResult mlir::applyPassManagerCLOptions(PassManager &pm) {
// Add the IR printing instrumentation.
options->addPrinterInstrumentation(pm);
+
+ if (options->generateReproducerFile.getNumOccurrences())
+ pm.enableGeneratePassManagerReproducer(options->generateReproducerFile);
return success();
}
diff --git a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
index 30fd384f3977c..56932045a229d 100644
--- a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
+++ b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
@@ -198,15 +198,6 @@ struct MlirOptMainConfigCLOptions : public MlirOptMainConfig {
static cl::list<std::string> passPlugins(
"load-pass-plugin", cl::desc("Load passes from plugin library"));
- static cl::opt<std::string, /*ExternalStorage=*/true>
- generateReproducerFile(
- "mlir-generate-reproducer",
- llvm::cl::desc(
- "Generate an mlir reproducer at the provided filename"
- " (no crash required)"),
- cl::location(generateReproducerFileFlag), cl::init(""),
- cl::value_desc("filename"));
-
static cl::OptionCategory remarkCategory(
"Remark Options",
"Filter remarks by regular expression (llvm::Regex syntax).");
@@ -568,14 +559,6 @@ performActions(raw_ostream &os,
if (failed(pm.run(*op)))
return failure();
- // Generate reproducers if requested
- if (!config.getReproducerFilename().empty()) {
- StringRef anchorName = pm.getAnyOpAnchorName();
- const auto &passes = pm.getPasses();
- makeReproducer(anchorName, passes, op.get(),
- config.getReproducerFilename());
- }
-
// Print the output.
TimingScope outputTiming = timing.nest("Output");
if (config.shouldEmitBytecode()) {
|
jpienaar
reviewed
Sep 17, 2025
mlir/include/mlir/Pass/PassManager.h
Outdated
| @@ -282,6 +282,11 @@ class PassManager : public OpPassManager { | |||
| /// Add the provided instrumentation to the pass manager. | |||
| void addInstrumentation(std::unique_ptr<PassInstrumentation> pi); | |||
|
|
|||
| /// Enable or disable the printing of pass manager reproducer. | |||
| void enableGeneratePassManagerReproducer(std::string filename) { | |||
There was a problem hiding this comment.
OOC, what about a raw_ostream here? (making it a filename limits where one can create reproducers to where LLVM's file utilities work).
There was a problem hiding this comment.
It's tricky because it's pretty ingrained right now, see many layers down how it'll end-up to:
There was a problem hiding this comment.
We already have
one the pass manager. And that can be set with either stream or filename. So this is really about adding an option so that it would generate a reproducer even if no error occurred, is that correct? (the filename or stream setting is independent really). And you want that per pipeline? (e.g., in PassInstrumentation equivalent would be to generate in runBeforePipeline). And potentially additionally into a different place than where the crash reproducer would go?There was a problem hiding this comment.
So this is really about adding an option so that it would generate a reproducer even if no error occurred, is that correct? (the filename or stream setting is independent really).
Yes
Note: I don't add the option, it exists in mlir-opt, it just wasn't setup as a "pass manager CL options" and thus not available to non-mlir-opt binaries.
And you want that per pipeline?
It is per-invocation of the pass-manager itself. Not nested ones, not dynamic pipeline.
There was a problem hiding this comment.
I'd still prefer this to be uniform with enableCrashReproducerGeneration - I see the stream version as the real one, and the file one as convenience. For opt tool that was fine as it is a non-production/testing tool, but feels wrong to encode that and limit usability. I think one can even do this with PassInstrumentation still.
It is per-invocation of the pass-manager itself. Not nested ones, not dynamic pipeline.
I'm not sure why this isn't just a helper method on pass-manager that writes to ostream/produce string then.
E.g., pm.populateReproducer(os)/make makeReproducer more user friendly. The only benefit here is that its run per pass manager run, but I'd hesitate to say where I'd want that arbitrarily vs controlling it. And also you have to enable it on your passmanager of choice, so you already have to identify a pass manager, and then enable, then run and then have it dump. Vs at that same spot just serializing the reproducer even before run.
There was a problem hiding this comment.
Vs at that same spot just serializing the reproducer even before run.
It's not the same spot: there is a different between "setup code" and execution code in general (also: this prints the pass pipeline and the IR that will be processed together)
Right now the documentation we have state:
The pass manager in MLIR contains a builtin mechanism to generate reproducibles in the event of a crash, or a pass failure. This functionality can be enabled via
PassManager::enableCrashReproducerGenerationor via the command line flagmlir-pass-pipeline-crash-reproducer.
Using a stream is a non-starter, but I can add an indirection with a stream factory though.
There was a problem hiding this comment.
PTAL with the factory?
… option instead of mlir-opt This makes it available to compilers in general, not limited to mlir-opt-like tools.
joker-eph
force-pushed
the
reproducer
branch
from
e203b84 to
67bb4b1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This makes it available to compilers in general, not limited to mlir-opt-like tools.