[PATCH] D96535: [flang][fir] Add fir-opt tool

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added a comment.

Thanks, I wrote exactly this locally before seeing this patch! :)

Can you also update `test/Fir/fir-ops.fir` and `test/Fir/fir-types.fir` to use this? I had changed the run line this way:

  // Parse operations and check that we can reparse what we print.
  // RUN: fir-opt %s | fir-opt | FileCheck %s





================
Comment at: flang/tools/fir-opt/CMakeLists.txt:21
+  MLIRVectorToLLVM
+  MLIROptLib
+)
----------------
This list of dependency should be pruned.

These two are obvious:

```
  FIROptimizer
  MLIROptLib
```

None of the others are.
I suspect the issue is that  `fir::registerFIRPasses()` should not just be an inline function in a header but would better come in an implementation file, with a CMake target that pulls in the minimum number of dependencies to link the desired passes.
Otherwise any user of this function has to know the list of targets, and changing the function requires to change the build configuration of every user.


================
Comment at: flang/tools/fir-opt/fir-opt.cpp:23
+  registerAllDialects(registry);
+  registry.insert<fir::FIROpsDialect>();
+  return failed(MlirOptMain(argc, argv, "FIR modular optimizer driver\n",
----------------
Can you replace the two lines above with `  fir::registerFIRDialects(registry);` ?


================
Comment at: flang/tools/fir-opt/fir-opt.cpp:27
+}
\ No newline at end of file

----------------
(nit: missing newline)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96535/new/

https://reviews.llvm.org/D96535