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

[Valentin Clement via Phabricator via llvm-commits]

clementval marked 2 inline comments as done.
clementval added a comment.

In D96535#2557961 <https://reviews.llvm.org/D96535#2557961>, @mehdi_amini wrote:

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

Thanks for the review.

Sorry, I was a bit slow to submit the patch. When I saw your comment I suspect you might have started to work on it on your side as well.



================
Comment at: flang/tools/fir-opt/CMakeLists.txt:21
+  MLIRVectorToLLVM
+  MLIROptLib
+)
----------------
mehdi_amini wrote:
> 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.
I have pruned it to a minimum. I'll check with @schweitz so we refactor fir::registerFIRPasses(). 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96535