[PATCH] D123297: [flang][driver] Add support for -mmlir

awarzynski added inline comments. Comment at: clang/include/clang/Driver/Options.td:3272 MarshallingInfoStringVector>; +def mmlir : Separate<["-"], "mmlir">,Flags<[CoreOption,FC1Option,FlangOption]>, + HelpText<"Additional arguments to forward to MLIR's option processing">; -

[PATCH] D123297: [flang][driver] Add support for -mmlir

This revision was automatically updated to reflect the committed changes. Closed by commit rG6c93e1d329e6: [flang][driver] Add support for `-mmlir` (authored by awarzynski). Changed prior to commit: https://reviews.llvm.org/D123297?vs=422163&id=422787#toc Repository: rG LLVM Github Monorepo

[PATCH] D123297: [flang][driver] Add support for -mmlir

MaskRay added inline comments. Comment at: clang/include/clang/Driver/Options.td:3272 MarshallingInfoStringVector>; +def mmlir : Separate<["-"], "mmlir">,Flags<[CoreOption,FC1Option,FlangOption]>, + HelpText<"Additional arguments to forward to MLIR's option processing">;

[PATCH] D123297: [flang][driver] Add support for -mmlir

awarzynski added inline comments. Comment at: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:15 +#include "mlir/IR/MLIRContext.h" +#include "mlir/Pass/PassManager.h" #include "flang/Frontend/CompilerInstance.h" rovka wrote: > Nit: Should these come after t

[PATCH] D123297: [flang][driver] Add support for -mmlir

rovka accepted this revision. rovka added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123297/new/ https://reviews.llvm.org/D123297

[PATCH] D123297: [flang][driver] Add support for -mmlir

awarzynski updated this revision to Diff 422163. awarzynski added a comment. Rebase on top of `main`, re-fine the test Make sure that the test file correctly takes into account that `-mmlir -help` is a superset of `-mllvm -help`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D123297: [flang][driver] Add support for -mmlir

awarzynski added a comment. I did a bit of digging and realised that MLIR llvm::cl options are lazily constructed on demand (see the definition of options

[PATCH] D123297: [flang][driver] Add support for -mmlir

awarzynski added inline comments. Comment at: flang/test/Driver/mllvm_vs_mmlir.f90:17 +! MLLVM: flang (LLVM option parsing) [options] +! MLLVM: --print-ir-after-all +! MLLVM-NOT: --mlir-{{.*}} rovka wrote: > awarzynski wrote: > > awarzynski wrote: > > > rriddle w

[PATCH] D123297: [flang][driver] Add support for -mmlir

rovka added inline comments. Comment at: flang/test/Driver/mllvm_vs_mmlir.f90:17 +! MLLVM: flang (LLVM option parsing) [options] +! MLLVM: --print-ir-after-all +! MLLVM-NOT: --mlir-{{.*}} awarzynski wrote: > awarzynski wrote: > > rriddle wrote: > > > awarzynski w

[PATCH] D123297: [flang][driver] Add support for -mmlir

awarzynski added inline comments. Comment at: flang/test/Driver/mllvm_vs_mmlir.f90:17 +! MLLVM: flang (LLVM option parsing) [options] +! MLLVM: --print-ir-after-all +! MLLVM-NOT: --mlir-{{.*}} awarzynski wrote: > rriddle wrote: > > awarzynski wrote: > > > awarzyn

[PATCH] D123297: [flang][driver] Add support for -mmlir

awarzynski added a comment. In D123297#3436867 , @mehdi_amini wrote: > `-mmlir` is fine with me, but note that MLIR has much less global options > than LLVM: you will only get access to context and passmanager options and > not individual passes flags.

[PATCH] D123297: [flang][driver] Add support for -mmlir

mehdi_amini added a comment. `-mmlir` is fine with me, but note that MLIR has much less global options than LLVM: you will only get access to context and passmanager options and not individual passes flags. That's not a criticism :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D123297: [flang][driver] Add support for -mmlir

awarzynski added inline comments. Comment at: flang/test/Driver/mllvm_vs_mmlir.f90:17 +! MLLVM: flang (LLVM option parsing) [options] +! MLLVM: --print-ir-after-all +! MLLVM-NOT: --mlir-{{.*}} rriddle wrote: > awarzynski wrote: > > awarzynski wrote: > > > rovka w

[PATCH] D123297: [flang][driver] Add support for -mmlir

rriddle added inline comments. Comment at: flang/test/Driver/mllvm_vs_mmlir.f90:17 +! MLLVM: flang (LLVM option parsing) [options] +! MLLVM: --print-ir-after-all +! MLLVM-NOT: --mlir-{{.*}} awarzynski wrote: > awarzynski wrote: > > rovka wrote: > > > rovka wrote:

[PATCH] D123297: [flang][driver] Add support for -mmlir

awarzynski updated this revision to Diff 421223. awarzynski added a comment. Make sure the MLIR CL options are "applied" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123297/new/ https://reviews.llvm.org/D123297 Files: clang/include/clang/Driver

[PATCH] D123297: [flang][driver] Add support for -mmlir

awarzynski added a comment. In D123297#3435816 , @rovka wrote: > I don't know the command line library that well, so I have this curiosity: > what happens if LLVM and MLIR have 2 different options with the same name? Do > we get a compile time error? T

[PATCH] D123297: [flang][driver] Add support for -mmlir

awarzynski updated this revision to Diff 421168. awarzynski edited the summary of this revision. awarzynski added a comment. Fix pre-commit CI Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123297/new/ https://reviews.llvm.org/D123297 Files: clan

[PATCH] D123297: [flang][driver] Add support for -mmlir

rovka added inline comments. Comment at: flang/test/Driver/mllvm_vs_mmlir.f90:17 +! MLLVM: flang (LLVM option parsing) [options] +! MLLVM: --print-ir-after-all +! MLLVM-NOT: --mlir-{{.*}} rovka wrote: > Is this the most llvm-ish option we have? I'm concerned that

[PATCH] D123297: [flang][driver] Add support for -mmlir

rovka added a comment. I don't know the command line library that well, so I have this curiosity: what happens if LLVM and MLIR have 2 different options with the same name? Do we get a compile time error? Or is there a risk that someone might -mllvm -XYZ and it would end up in MLIR's XYZ option

[PATCH] D123297: [flang][driver] Add support for -mmlir

awarzynski created this revision. awarzynski added a reviewer: rovka. Herald added subscribers: sdasgup3, wenzhicui, wrengr, Chia-hungDuan, dcaballe, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, antiagainst, shau