Keep output file after successful execution of mlir-opt

[LukasSommerTu]

Summary

Invoke keep() on the output file of mlir-opt in case the invocation of MlirOptMain was successful, to make sure the output file is not deleted on exit from mlir-opt.
Fixes a similar problem in standalone-opt from the example for an out-of-tree, standalone MLIR dialect.

This revision also adds a missing parameter to the invocation of MlirOptMain in standalone-opt.

[ftynse]

Can we rather do it inside MlirOptMain instead of repeating the code?

[LukasSommerTu]

In the current design not. MlirOptMain does not receive the file itself, but rather an output stream, so it cannot manage the file.

We could of course change the signature of MlirOptMain to accept a ToolOutputFile instead, but that would loose some generality, because one would always have to create a ToolOutputFile for invoking MlirOptMain instead of the more general raw_ostream. Alternatively, we could add another method to MlirOptMain.cpp that manages the file and internally invokes the original MlirOptMain again.

What would you prefer?

[Kayjukh]

mlir/examples/standalone/standalone-opt/standalone-opt.cpp

49

Should this be part of a separate patch? It is not really related to your proposed change.

[Kayjukh]

I think it would make sense to propagate the changes to mlir-opt as well.

[LukasSommerTu]

In D77643#1967140, @Kayjukh wrote:

I think it would make sense to propagate the changes to mlir-opt as well.

The changes to mlir-opt.cpp propagate the change of the file-management to mlir-opt. Or did you refer to something else?

mlir/examples/standalone/standalone-opt/standalone-opt.cpp

49

I can make a separate patch for that, but then we would need to pass a magic constant true or false to MlirOptMain, to match the number of required parameters.

But if you prefer to have that in a separate patch, I can extract it. What do you prefer?

[Kayjukh]

mlir/examples/standalone/standalone-opt/standalone-opt.cpp

49

I was thinking about making a patch for this command line argument first, and then adding the call to keep. At least I would update the title and description to mention these changes.

[Kayjukh]

In D77643#1967192, @LukasSommerTu wrote:

In D77643#1967140, @Kayjukh wrote:

I think it would make sense to propagate the changes to mlir-opt as well.

The changes to mlir-opt.cpp propagate the change of the file-management to mlir-opt. Or did you refer to something else?

Nvm, for some reason I overlooked the changes to mlir-opt.cpp.

[LukasSommerTu]

@mehdi_amini: I do not have commit access, could you please commit the revision for me?

mlir/examples/standalone/standalone-opt/standalone-opt.cpp

49

I've updated the description to state the changes to standalone-opt more clearly.

mlir/examples/standalone/standalone-opt/standalone-opt.cpp

llvm::cl::desc("Check that emitted diagnostics match "
                    "expected-* lines on the corresponding line"),
     llvm::cl::init(false));
 
 static llvm::cl::opt<bool> verifyPasses(
     "verify-each",
     llvm::cl::desc("Run the verifier after each transformation pass"),
     llvm::cl::init(true));
 
+static llvm::cl::opt<bool> allowUnregisteredDialects(
+    "allow-unregistered-dialect",
+    llvm::cl::desc("Allow operation with no registered dialects"),
+    llvm::cl::init(false));
+
 static llvm::cl::opt<bool>
     showDialects("show-dialects",
                  llvm::cl::desc("Print the list of registered dialects"),
                  llvm::cl::init(false));
 
 int main(int argc, char **argv) {
   mlir::registerAllDialects();
   mlir::registerAllPasses();
int main(int argc, char **argv) {
   }
 
   auto output = mlir::openOutputFile(outputFilename, &errorMessage);
   if (!output) {
     llvm::errs() << errorMessage << "\n";
     exit(1);
   }
 
-  return failed(mlir::MlirOptMain(output->os(), std::move(file), passPipeline,
-                                  splitInputFile, verifyDiagnostics,
-                                  verifyPasses));
+  if (failed(MlirOptMain(output->os(), std::move(file), passPipeline,
+                         splitInputFile, verifyDiagnostics, verifyPasses,
+                         allowUnregisteredDialects))) {
+    return 1;
+  }
+  // Keep the output file if the invocation of MlirOptMain was successful.
+  output->keep();
+  return 0;
 }

mlir/test/mlir-opt/outputfile.mlir

+// RUN: mlir-opt %s -o %t
+// RUN: test -f %t

mlir/tools/mlir-opt/mlir-opt.cpp

int main(int argc, char **argv) {
   }
 
   auto output = openOutputFile(outputFilename, &errorMessage);
   if (!output) {
     llvm::errs() << errorMessage << "\n";
     exit(1);
   }
 
-  return failed(MlirOptMain(output->os(), std::move(file), passPipeline,
-                            splitInputFile, verifyDiagnostics, verifyPasses,
-                            allowUnregisteredDialects));
+  if (failed(MlirOptMain(output->os(), std::move(file), passPipeline,
+                         allowUnregisteredDialects))) {
+    return 1;
+  }
+  // Keep the output file if the invocation of MlirOptMain was successful.
+  output->keep();
+  return 0;
 }